Hacker News Reader

Comment by 321abc | Hacker News Reader

The author lists the following as a symptom of being "unable to reason about code":

"5. 'Bulldozer code' that gives the appearance of refactoring by breaking out chunks into subroutines, but that are impossible to reuse in another context (very high cohesion)"

I disagree with this. The purpose of breaking out code in to subroutines isn't only for code reuse. If you don't break out code in to subroutines, at some point your code is going to become unwieldy and difficult to read.

I try to stick to the rule of having each subroutine consist of no more than a page of code (so that each subroutine can be seen all at one time, without needing to scroll). Often that leads me to break up the subroutine in to many other subroutines. This improves readability greatly and makes it much easier to reason about the code. If any of the subroutines I break my code out in to happen to be reusable, that's just icing on the cake, not a necessity.

View on HN

A precedural decomposition should at least partition your code along meaningful lines so that a reader can potentially find something faster or understand its functionality in simpler terms. If they have to jump all over the place to make any sense of your code, you're just obfuscating.

I've seen plenty of bad code where a task is broken down arbitrarily, creating sets of procedures like foo, begin_foo, execute_foo, actually_do_foo, foo_part_two, etc. This seems to be a particular problem with OOP, where tasks have to be handed off from object to object, down the chain of responsibility.

it is not a problem with OOP so much as it is with the abuse of OOP, which is easy to do. Bad developers put the work flow procedures in objects and treat it as if it can be used as traditional OOP. but the truth is that work flow is usually task specific and not reusable. It lends itself better to procedural programming. Good OOP programmers know to stuff this stuff in thread safe static subroutines(methods) and provide a service layer for all work flow. it should not be embedded in objects (i know technically the static methods are in an object). Unfortunately this has also caused another bad OO problem with anemic objects. If you look at MVC implementations like struts it has caused a new bad habit in which objects are used for nothing more than structures and all of the logic is implemented in the work flow. Reusable business logic should be implemented as methods on the object in which they relate to. User specific work flow should be abstracted away from the objects and the UI should be loosely coupled to the work flow through a service facade. Alternatively if you have a group of good OO programs you can implement the work flow elements in a reusable event based system where reusable paths can be developed and listeners can chain in for very specific non-reusable tasks, but this is very hard for a junior to follow so many good developers opt for the first pattern because it allows them to train new developers on the system, without having to teach a programmer event based techniques. I don't know why it is so hard for people to grasp OO, but it does seem like it is much easier to create a hideous unwieldy beast with it, than procedural, in the hand of the wrong developer. Conversely, some of the most elegant systems that I have seen where done with OO languages and properer delineation between the data systems the business model and logic, the work flow and the interface.

Anyway, long story short when I see a huge stack chain, general they are either do anemic objects and just hacking up the big controller procedure into a bunch of little functions or they are doing OO work flow, either of which is not an appetizing prospect to support or fix. Evey once and a while you will get someone that is doing factory patterns everywhere which can create a lot of deep dives down the stack to figure out that the 17 methods deep stack finally results in one method creating a object.

P.S. Private subroutines do not have to be reusable public ones should be. The author did not specify. so I hope and assume he was talking about public subroutines.

I thought this as well when I was reading it.

I think what the article is suggesting, though, is not just that the subroutines would be "reusable" as in useful if called from another part of the program.

He's suggesting that the contents of each subroutine should be decoupled from each other - so not sharing global state, depending on strange changes made in other seemingly unrelated methods, etc.

The property of "I could reuse this routine if I needed to" implies low cohesion and therefore (probably) more maintainable code.

Steve Yegge argued something related to this topic that I don't think I like much, and I don't think you'd agree with it either. He claimed that junior programmers tend to write shorter methods because they're not used to fitting large chunks of complex code in their heads: http://steve-yegge.blogspot.com/2008/02/portrait-of-n00b.htm...

As someone who has seen 2000-line long C++ methods written by "senior developers", I still think short clearly named methods are better. :)

I have seen Fortran programs that don't have any subroutine at all //senior guru model. Even if you could reason about that large amount (40K LOC) I don't enjoy when you die and I get the responsibility to fix "the bug".

I do agree that you should not (depending on language) write sub of everything. ex int add(int a, int b) {return a+b;} //Junior model

Sometimes the (seemingly) simple act of trying to come up with a name for a subroutine helps me reason about what it is I'm trying to do (whether the chunk is re-usable or not).

Not being able to think of a good name for a variable, function, class or data structure is a show-stopper for me. If I can't come up with a short, descriptive name, then I assume I don't really understand what I want that thing to do or be. I'll sit and think until I'm satisfied I have both a meaningful name and a grasp on how this thing fits into the problem.

I don't know if you've tried this, but if you write down some pseudocode before you really get started, the logical chunks of the program will take their names straight from the pseudocode.

Or do comment-driven development: write the comments for the method first, then convert the (hopefully!) plain English into code, ideally so that no comment is needed.

Am I the only one that thinks the original author's use of the term cohesion is wrong? I thought the goals were: high cohesion and loose coupling.

http://en.wikipedia.org/wiki/Cohesion_(computer_science) http://en.wikipedia.org/wiki/Coupling_(computer_science)

I'm surprised anyone noticed this. Yes, I agree.

I have done this too, particularly in small projects. One thing I try to do in these situations is imagine what functions would be useful should I extend the program later. Anther trick is to imagine a different pardigm, i.e. coding it up as a state machine.

What I gathered from the original article however, is a focus on those guys who will decompose long_function(params) into short_func1(params), the last line of which is return short_func2(params, 12 state params); .

I'm not fond of scrolling (or ctrl+clicking) up and down to see what every little subroutine does to understand the code I have to fix. Only if it's my own code, I can be somewhat sure the tiny function does indeed SetOrderDate, but maybe it also modifies order's XML or changes some global variables... I can never be sure and I have to look, so reading bulldozed code is almost like reading huge routine but with way more scrolling.

Using ctags and a good editor (or an IDE) will make the process of seeking to (and jumping back from) subroutines a lot less painful.

If readable chunks were all you cared about, why don't you just insert a few newlines and a single-line comment? I bet you actually do take some care to reduce coupling even when not intending reuse.

I agree with you, but I think that when it is possible to break it into atomic functions that are reusable then that is superior to doing it simply for size reasons.

Reply on news.ycombinator.com