Hacker News new | ask | show | jobs
by yebyen 2311 days ago
Is it a method that's longer than 5 lines, or is it a method that is longer than 50? Because one of these things I could see being easily explained away without haranguing, but the other one is what I deal with on my team on a regular basis.

The point I was trying to make isn't that your methods shouldn't be longer than 5 lines, it's that they should be single-responsibility and descriptively named, like the well-designed classes they inhabit.

The 50+ line method which mixes query and command behaviors indiscriminately is the problem. The 5 line method rule is simply one possible distance to the goalposts (and you are free to move the goalpost, no bullshit intended!)

The point is not that methods should be 5 lines or less, full stop. The point is that people who are in the habit of writing methods that are above (threshold value X) ... and leave it this way, boilerplate and all, then spam the same 50 lines everywhere, except on Tuesday when it looks like this other variant... are perhaps contributing to some kind of a trouble state that should trigger at least a second glance whenever your process has you get around to doing code reviews. It's likely one of the same reasons you or your team has reached for this tool to begin with.

If the effect of implementing the tool into your process is that people are still squeezing command and query ideas that don't belong together into the same method, now in fewer lines than ever before, then it's not having all of the desired effect and it sounds like there is still a conversation that needs to happen about what is a good and bad design for a method.

I'm not saying that your personal boilerplate is wrong, (but I am suggesting it might be, based on a metric that is easy to compute.)

It's a tool that you use to identify symptoms of a problem. The symptoms are not the problem, and they may not even be indicative of any real problem. Sometimes you get a sniffle.

1 comments

> The point is not that methods should be 5 lines or less, full stop.

But that's what a linter does! It just blindly says that shit is wrong. Linters should only emit warnings that people definitely should fix otherwise people stop paying attention and all the useful checks become just noise.

I'm not saying that this rule is the most beautiful rule, or that you're a bad person if you write methods that are longer than 5 lines and don't get permission from a senior architect first. That's something dysfunctional teams might do.

Look at `--auto-gen-config` because you can totally still use rubocop on codebases that have loads of pre-existing violations in them without fanfare. Then each new violation needs a second look, and an addition in this file. And if the rule is too restrictive, you can change the default away from 5 lines to some bigger number. The principle I hope (Rubocop hopes) you will accept is that short methods are easier to understand than long methods, so shorter methods should be preferred, at least absent other pressures... that's all. The point I guess is to establish a standard, and then iterate on it.

I don't want to start name calling, but Sandi does this talk where she starts out "who knows code smells" and everyone puts their hand up, then she says "who can name 5 code smells" and all the hands go down. Watching this talk was eye opener for me.

Can you write a program that is well organized and doesn't have methods longer than 5 lines? I don't know if I can. Does that mean the metric is bad and should be thrown away without looking back? I'm not ready to go that far, I want to learn more and know how to make this possible, because Sandi and many others with 30+ years experience in OOP have taken time out from their busy day to suggest that it will result in more maintainable code for my team.

There is a big difference between cargo culting and listening to learned experience projected outwardly.

Edit: https://www.youtube.com/watch?v=PJjHfa5yxlU This is the talk about code smells! "Get a Whiff of This" by Sandi Metz, RailsConf 2016