Comment by mr_crankypants | Hacker News Reader

Comment by mr_crankypants | Hacker News Reader

Parent Full thread

Personally, yeah, I agree, my belief is that most the really long functions I've encountered have been awful unreadable bug farms. But I still think it's a bad idea for a static analysis rule. Two reasons:

1. Static analysis tools should not generate useless noise. I am already well aware that that function I just edited was 1,000 lines long. If I didn't break it down, it's either because I didn't think doing so would be a good idea, or because I don't perceive that to be a problem in the first place, or because I simply don't care. In any case, there's a 100% chance that I'm going to ignore any advice from the static analysis tool on this one. That means that such a rule is, at best, useless.

2. I'm generally not sure if my feelings about long functions reflect reality. I recently saw a talk about evidence-based software practice where the speaker claimed that nobody's been able to demonstrate a link between average function length and code quality, and not for lack of trying.

I think it's useful if you have a rule at the outset of a project that rejects things longer than X where X is the 99% percentile length of a method.

I ran the stats retrospectively on a codebase I once worked on to derive sensible values for max class length and max function length. To be honest, they came out at some fairly sensible values and I would be happy putting a blocker in place to say "if you are doing anything more than this, you shouldn't be". I think it was something like 200 lines for a method and 3 or 4 thousand lines for a class.

If you can do it at the outset what you'll find is that the vast majority of the time the rule is never triggered. In the majority of cases that it is triggered, it's trivially easy to break the method up a small amount to be under the limit. It's very, very rare to feel like you're having to break it up unnaturally and it feels very forced, though it happens on the extremely odd occasion. I'm OK with that trade-off. The benefit is you never wind up with the 5k line long methodzilla that someone thinks is a good idea and everyone laments forever.

Every code base I've seen has one godzilla method. I'd prefer we avoid godzilla methods at least. You don't need an 18k line class. Nor a 5k line method.

Yeah. It's not necessarily that I don't think that it's useful to place limits on length. It's more that it's hard for me to imagine a situation where a godzilla method like that sails past code review, but then gets broken up, anyway, because the static analysis tool said, "Oh, hey, getting a little big there." A static analysis tool is best when it's used for catching things that are hard to see. I'd argue that employing it as a substitute for caring about your code is more of a misuse. And that using it as a substitute for a culture of careful, diligent code review is just a special case of that.

Among every team I've worked on there have been the devs that have a real eye for detail and truly care about code quality and put real thought into their work. And there are the ones who are just trying to get paid and don't really think too much about it. I trust the former, I don't trust the latter.

The point is the static analysis tool fails the build, so it becomes physically impossible for the godzilla method to exist. I trust the machine to produce a predictable result. People are a huge source of entropy and having a little bit of automation to help contain some of that entropy I think is a very wise use of technology in CONJUCTION with promoting a culture of caring about quality and diligent review.

It's hard to imagine how anyone ever lets godzilla methods and god objects and all other kinds of code smells become a thing... and yet... they're a real phenomenon that seem to make their way into large code bases at some point or other.

And I guess my concern is that doing something like this is probably solving the wrong problem.

I've worked with people who write messy, tangled code in giant methods, and my experience has been that forcing them to write smaller functions is futile. Satisfying the static analyzer is just too easy: Move all your variables into mutable fields that all the methods fiddle with, break the contents of the method out into smaller methods with cryptic and misleading names, create a master method that calls them all in a row. Voila, with barely any effort you've satisfied the static analyzer and made the code even less maintainable.

Ah OK. Point taken. Yeah... that's a thorny issue. I still prefer both measures. I don't see whats wrong with relying on all tools in your toolbox than preferring one over the other.

Human factors always mess up software... _sigh_

Reply on news.ycombinator.com