[mumblemumble]

Meh. If you're only doing it once, then I'd argue that the main practical difference between

  // Add a horizontal scroll bar
  hScrollBar = new JScrollBar(scrollBar, HORIZONTAL);
  add(hScrollBar, BorderLayout.SOUTH);

and (assuming scrollBar is not a local variable)

  void addHorizontalScrollBar() {
      hScrollBar = new JScrollBar(scrollBar, HORIZONTAL);
      add(hScrollBar, BorderLayout.SOUTH);
  }

  ...

  addHorizontalScrollBar();

is whether you want to do it in 3 lines or 5.

(edit: disclaimer -- Snark aside, I should say that I personally tend toward the latter option. But I am absolutely not prepared to try and convince anyone that it is, in any objective sense, better. It's just an aesthetic preference. Maybe even a nervous habit.)

[tharkun__]

I think this is a question of level of abstraction.

I would argue that that this more readable as for what is going on:

    addHorizontalScrollbar() ;
    addVerticalScrollbar();
    addHeader();
    addDayColums();
    addFooter();

If all I need to know is how the page is structured and set up on a high level, then this is awesome (actually I'm not convinced it needs the scroll bar stuff on this level but this was what I could come up with to stay in the example :) ) I don't need to read comments for what's what and then manually skip over ever so many lines of actual code that did all these things. I can decide that what I really wanted to take a closer look at was what the header bar looks like.

Some of this depends on whether I'm using an IDE or not. In an IDE I can just Ctrl click myself through. Then again I doubt anyone these days is using _just_ vi to code something complex enough to want multiple files. I used vi configured as an IDE way back but I do admit that's been like 15 years. Dunno what that looks like today. Nowadays I'm a JetBrains user, mainly in Java and Java/TypeScript and very little Python.

[js8]

This might be more readable if setting up those things doesn't involve a set of common variables (say a GUI style in this example) that can affect them. If it does, then you have to pass them around (into these functions) and it makes more difficult to figure out, if one of these variables changes, what all is affected by it (you have to look up all those methods).

In short, I don't think adding depth to a hierarchy of function composition is always an answer. Flatter hierarchies can be easier to understand too.

[Izkata]

I think it makes the source of such a styling bug/flow of what's affected a lot more obvious than having to find the reference in each line of the implementation details:

  addHorizontalScrollbar($style);
  addVerticalScrollbar($style);
  addHeader();
  addDayColums($style);
  addFooter($style);

[perl4ever]

>setting up those things doesn't involve a set of common variables

So...object methods?

[js8]

So what with them? I don't see how they solve the problem.

What you seem to suggest is refactor the original code (which presumably was a single function to set everything up) so that variables used and the substeps are in one module (your new object), and the code that uses all of this is in another module. I am not convinced this really follows "high cohesion, low coupling" dogma, since you put together things that might be less related (different style variables) into the same module, and created a function, which almost all it does is calling into another module, and is highly dependent on it.

The problem ultimately is data relationships in the code can be a general graph, and trying to put a general graph into a hierarchical structure (much less one that actually has to follow the structure of operations on the data) always has to break cycles in some way. And how much you can really do it depends on cyclomatic number of the graph, and if it's high, no strategy is going to be good. AFAICT most of the OOP strategies of "dealing" with this problem are just creating more nodes in the graph, making the structure bigger, but not really reducing the cyclomatic number.

[tester34]

and that's how we created $company_framework/lib, I guess?

[stickfigure]

The whole thing should be condensed to:

    add(new JScrollBar(scrollBar, HORIZONTAL), BorderLayout.SOUTH);

It's self-evident and needs no comment or whitespace to "break it up". This is fundamentally a problem with wordy imperative languages.

A lifetime ago when I wrote assembly code I would write a comment like this to explain every subsequent 5-10 lines of opaque incantation. Higher level (especially functional) languages tend to have fewer issues like this.

[nemetroid]

Unlike the original, this doesn't retain a reference to the scrollbar object.

[emodendroket]

I'd say it's the enforcement. It's like how YARD docs convey the same information as built-in compile-time type declarations, except you can't trust them. So you start ignoring them, forget to update them yourself, and kick off a vicious cycle.

[mattacular]

This is an often used example in defense of not writing comments but in practice I rarely have encountered an out of date or misleading inline comment. In my experience, everything in code tends to stay up-to-date because it is often written and reviewed by multiple people who have some incentive to keep it correct. It's other documentation outside of code that frequently goes stale.

[cutemonster]

Also, function names sometimes get out of date too, 'addHorizontalScrollBar()' in the example above -- let's say someone adds vertical scroll too but forgets to update the function name.

[Jetrel]

Having had a lot of experience myself, I sadly can't echo this. A lot of it has to do with stick-and-carrot incentivization, stack ranking, and other stuff.

Unfortunately every place I've worked at sort of accidentally pits their programmers against each other in a sort of "who can get it done first" competition, with those who do a quick-and-dirty job getting credit, and those who clean up their proverbial workbench getting none. We've had gentlemen's agreements to not do this, and have even been able to collectively oust (i.e. get fired) a few serial violators, but there's just SUCH pressure to ship quickly that code review is basically a fictional thing like the flying spaghetti monster.

It's telling that we've got a MLOC codebase, and basically not a single piece of documentation - and every single place I've worked, in almost 2 decades, has been exactly like this.

Enterprise software is a wild ride. :|

[IlliOnato]

I would not agree to work in a place like you describe if they doubled my current salary.

I really hope these companies compensated you well...

[emodendroket]

Consider yourself lucky then. I've wasted tons of time because of them.

[drran]

Consider reviewing of your code review process.

[emodendroket]

Code review can miss because the inaccurate comments might not be close enough to show up in the diff viewer

Generally if I see something happen in every job I've had throughout my career I start to doubt that there is a simple process solution.

[drran]

Confirmed.

We found a problem! Let start a startup: "Intelligent code review tool for smart developers, which want to keep comments in their code up to date". The tool will use AI to learn which parts of code are modified often in unison, and then propose to check them also during code review. It will also check for copy-pasted code blocks, and for snippets from StackOverflow with known bugs. The tool will reduce code review time by 10%, reduce number of bugs slipped trough code review by 20%, improve time to ship by 7%, bla bla bla, and instruct the coffee machine to prepare a coffee just in time for the review.

[mattacular]

I guess I have been lucky but I don't really get this. You read comments along with the actual code right? They complement one another. An out of date comment should not ipso facto render code indecipherable and if it does that speaks to a bigger problem with the code itself.

I find it strange that people want to throw the baby out with the bathwater when it comes to commenting code just cause they've been bit before.

[emodendroket]

You don’t see how you could end up wasting time when a comment led to you making an incorrect assumption? If they don’t affect how you read the code would mean they’re useless.

[mattacular]

I can see how it could, in theory, if the code is extremely complicated and then the comments are lying to you on top. Again I have never experienced this or anything even close to it in 1.5 decades of coding on teams. Unless every comment is wrong then some of them are useful to someone.

On the other hand, I have been bit plenty of times by incorrect documentation outside of code about libraries and APIs.

[OJFord]

But now you can also easily test it independently of whatever else is there if required, and the 'section heading' comment (now function name) is much less likely to go out of sync with the actual code, or accidentally adopt unrelated GUI setup that should have been in a different 'section'.