Hacker News new | past | comments | ask | show | jobs | submit login

> PS. If you happen to be a developer of a static analysis tool: Consider leaving long functions alone, rather then reporting them as suspicious. There are circumstances where long functions are perfectly legal and desirable.

Sure, _there are_ circumstances. They're just extremely rare and their occurrence is dependent on the team as well. So generally, I'd say function length a pretty good proxy for complexity--as long as there are overrides.

I disagree with this. I think something should be a seperate function only if it is (or likely will be) called from several callsites.

If you try to not always think about contrived cases while writing code, then this rule can sometimes lead to quite long functions simply from executing steps in sequence ("script").

One can split what is really a long function into several small ones for readability, but if one does so I think one should declare that function within the calling function (in languages that support this), and whether one does that or not is really about as insignificant as tab vs spaces or where one likes to have curly braces.

My point is, if a function only has one callsite, and is not likely to get more (incl. test code), then whether to inline that function or write it seperately is a style/formatting issue, not a maintainability issue.

> I disagree with this. I think something should be a seperate function only if it is (or likely will be) called from several callsites.

I disagree. I split out anything that I can describe/test separately. If I need to take a list and

1. Filter it based on custom logic 2. Sort it based on custom logic

Then I will generally break the filter and sort out into their own functions even if they're not used anywhere else. Each function can have it's own description and tests.

I understand that many prefer this. My point wasn't really so much mine or your preference -- my point was that it is a question of stylistic preference, not about spaghetti vs not.

As you simply describe your own stylistic preference I am not sure if you agree with me about that or not?

If you actually write a seperate test for the function then you have two callsites and of course it must be a seperate function.

Even if you have a few callsites and just copy-paste the same function it can be readable - I've heard that called un-cooked spaghetti code. Everything is still nicely layed out and can be refactored without too much trouble if you so chose. The problem starts with cooked spaghetti code where everything is all mangled up and references are all over the place.

I don't know if I'm buying it or not but I like this cooked/uncooked spaghetti code distinction

I prefer long functions for readability. I think it depends on domain. Carmack said something like no longer than 6k and that rule of thumb seems large for business code but apparently it is about right for games. I know the intellisense in visual studio breaks down around line 12k in a single cpp file. I probably shouldn't know that. Making changes in th other half of that file was annoying...

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_

Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | Legal | Apply to YC | Contact