Ruby - rubocop Method too long

I just set up VS Code for debugging (I was previously using Netbeans). I also set up auto formatting and linting using rubocop. I should have been using this a long time ago. My project shows almost 16K warnings.

It was very educational but there are a few cops that don’t make sense to me. Specifically rules for length of modules, classes, and methods.

https://rubocop.readthedocs.io/en/latest/cops_metrics/#metricsmethodlength

Seriously I can’t draw this door in a method of only ten lines. May be I should have a separate method to draw each part of the door? But then I can’t even draw it on a class or module of only 100 lines.


What is the proper way to do this? I realize I should probably have a class with properties describing the door, and then have a draw_door method rather rather than passing all the properties to the method.

How can I make my code more manageable?

@medeek How do you handle this? I’m sure you have some long… calculations :wink:

1 Like

You could perhaps extract things in smaller methods that can also be reused? draw_roof_thingy, draw_door, draw_window etc. You can also reconfigure that specific cop. In many projects I allow 20 line methods, even though i aim for 10 or less.

My rule of thumb is that if something can be given a meaningful name, i.e. doesn’t only make sense within a specific context, it can be extracted. I guess you could say the same about what could be a component in a SketchUp model.

These “rules” are someone else’s idea of how your code should be more readable for someone else. (ie, open source projects, etc.)

I never pay any attention to the 10 line method “rule”. It’s just an arbitrary amount of lines.

Now you are showing that you’ve got a single method that is over 1000 lines! Yea, that is excessive.

As I’ve said in the past (to either yourself or Nate,) learn to leverage Hashes (or OpenStructs) for holding properties and passing them around, (including to and from web dialogs via JSON strings.)

It looks like there is 4 or 5 parts to the door. It might be split up into these parts (ie a method for each.)
Drawing the sliders might be split up into submethods as well.


First I’ve heard of the 100 line module “rule” however. That I feel is also arbitrary. You could break up the module into separate files (which I often do to make it easier to work on. I separate methods into groups that do like things, for example all the observer callbacks might be in one file. All the methods that handle plugin options in another file, all the UI methods in another, etc.)

1 Like

I do already have my module broke out into 26 files to make it easier to navigate.

There are about a dozen different door types, and some of the door types use the same code for certain parts of the door. I could use sub methods for some parts and pass around the instance to the sub methods.

Thanks for this info. I’ll look up hashes and openstructs to see how they work.

I already have entry_door, overhead_door, and `barn_door’, methods. I could probably split it up more like draw_track, draw_left_panel… but the methods wouldn’t be reused in this case.

Heavily use of hashes is a typical sign that object orientation isn’t fully utilized. Often the content of such a hash can be properties of the object. In my experience the code gets less cluttered and more easily readable when you can access these values directly by name instead of having to specify both the hash and the key. Sending to a HtmlDialog is an exception as we are now transcending languages/environments and can’t send custom objects.

2 Likes

Rubocop has recommendations, which do not always make sense.

For such things as statically defined constant geometry, you might have a file with some large structures. A hash can be memory intensive, and you might consider creating a factory pattern to produce what you need, that would be easier to maintain in the future.

2 Likes

This is very true when working with low level geometry. You may need a lot of lines just to define all the coordinates for corners of an object. Rubocop is written with more high level things in mind, like users carrying out actions in a web service where the atom building blocks aren’t as large.

1 Like

I don’t worry about the rules (10 lines or whatever).

Like Eneroth has previously stated I find it useful to create methods that deal with things that are used over and over again.

For instance you might have a lot of door types and certain doors may include windows. Rather than have a separate method for every door’s windows it makes more sense to have a separate window method(s) that can be utilized and reused by various door methods.

This same methodology applies to other sub-units of a door (ie. door handles, trim, raised panels etc…)

Sometimes I create a whole module before it becomes apparent to me that I should pull things apart into separate methods. This usually happens when I notice the same or similar block of code being utilized in multiple places. Then it is just a matter of juggling things around a bit, creating a new method or two and compressing the code. It is always fun to see a bunch of code go from a bloated mess to a few lines of streamlined logic, I can’t say I’m always this lucky but that is the goal and standard I try to hold myself to.

I don’t consider myself a true wizard of ruby like Eneroth and Dan but I do manage to get by. As they say I know enough to be dangerous.

1 Like

That is the conclusion I was coming to. There are cases where a method limit simply doesn’t make sense.

Rubocop has a wide variety of configuration settings you can tweak to match your desired or required style. Though the areas it examines are well-known indicators of good and bad code, there is no reason to timidly accept someone else’s notion of where the boundaries lie. Take them as suggestions for what you might improve, not as absolutes.

Long ago I worked on a code project in which the manager decreed a style guide and established a review team to enforce compliance. The maximum function length mandate caused people to break perfectly clear and logical linear sequences of code into multiple function calls just to stay within the limit, even if there was tight dependency between each hunk and the others and even if there was zero chance the function would be useful anywhere else. I don’t think it led to better code in any useful sense.

2 Likes

A meta-programming coding exercise (from Aug 2018) in creating a data class factory …

Avoids the pitfalls of Struct, and as easy to use as OpenStruct (and avoids it’s quirks as it doesn’t use an internal Hash nor override the class object’s method_missing callback.)

1 Like

If you only care about the SketchUp suggestion you can disable all the default rules RuboCop comes with. It’s described here:

require: rubocop-sketchup

AllCops:
  DisabledByDefault: true
  DisplayStyleGuide: true
  SketchUp:
    SourcePath: src # Path to extension sources in project directory.
    EncryptedExtension: false # Enable if you plan to encrypt your extension.
    TargetSketchUpVersion: 2016 M1

SketchupDeprecations:
  Enabled: true

SketchupPerformance:
  Enabled: true

SketchupRequirements:
  Enabled: true

SketchupSuggestions:
  Enabled: true

SketchupBugs:
  Enabled: true

RuboCop is very opinionated. Some things are way too subjective for my taste. I use a relaxed version, which you can see in the rubocop-sketchup repository: https://github.com/SketchUp/rubocop-sketchup/blob/master/sketchup-style.yml

Because I work on a bunch of projects I’ve started to maintain that config file as the main style guide. RuboCop allows you to refer to external config files via URL. See TrueBend as an example:

There are other relaxed style guides out there, this one for instance:
https://relaxed.ruby.style/

Agree. Though I still have a max-cap set to 20 or 30 to make me think if I end up with methods larger than this. If they warrant it I’ll disable the warning locally for the method only.

Linters are very powerful, and very useful to enforce consistent styles across a code base. But never accept the default blindly. Especially if you are retro-fitting such a tool to an existing code base if can be a large amount of work to make it comply. Evaluate what these tools suggest and turn off what you judge to not be beneficial for you. (This is the reason the example config for rubocop-sketchup turns off all other default cops - it can be overwhelming.)

2 Likes

Thanks a bunch! That is exactly what I was looking for. Since you already did all that work there is no sense for me to re-invent the wheel!

Well, I have my own opinions about code style… :wink:

What I did try to make a point of was to explain why I chose not to use some of the default config from RuboCop. So people can make their own judgement about my own reasoning.

# 🖤 Unicode!

:heart: :smile:

1 Like

Your comments in the file are great. It’s a good starting point for someone new to rubocop.