Re^3: The Boy Scout Rule

[choroba]

You can find some interesting parts in my questions and meditations in the last year. However, most of the code is, unfortunately, agonisingly boring.

[BrowserUk]

You can find some interesting parts in my questions and meditations in the last year.

Hm. Over the past year you (appear to) have posted(*) 85 top-level SoPWs and no meditations:

Found 5 nodes roughly between 2013-01-01 and 1999-10-04 (searched 90.7
+2% of DB).


written by any of choroba

2012-08-06    choroba    Substring giving strange results on $1 with u
+tf8    SoPW
2012-04-22    choroba    Failing test in the Dancer application    SoP
+W
2012-02-11    choroba    Failing Tests in MozRepl    SoPW
2011-04-21    choroba    Relative URI    SoPW
2010-10-28    choroba    Tk: Cannot install from CPAN sometimes.    So
+PW



Finished searching database. 

Link to preload this search: [href://?node_id=3989;a=choroba;yr=2013;m
+o=01;dy=01;re=N;Wi;Tu]

of those, only the first stands out as being a possible candidate for your description above; and even that wasn't really so bad. Clumbsy maybe; but far from understandable and showing no signs of any particular fragility.

Ignore this. The post I was referring to (as returned by my first search), isn't returned by the second attempt, after the preloaded link failed. Super search is inconsistent and illogical.

However, most of the code is, unfortunately, agonisingly boring.

Boring's fine, and irrelevant to my inquiry. I'm interested in seeing samples of code that draw such vehement condemnation.

* I would have posted the preload link, but it didn't work; and the first time I did the search it returned 8 results; but when the link didn't work and I had to reconstruct the search, it only turned up 5. There's nothing like consistency; and that's ... :)

[choroba]

OK. Just some random samples. There are 150k more lines in the project.
Read more... (8 kB)

[Your Mother]

Oh… Hmmm… I was fixing to complain about the rows to hoe in my code base. But, uh, …nah… Uncle.

(Though truthfully ours is identical in spirit, just rarely deeper than 4 nested ifs or 25K lines per disaster.)

[BrowserUk]

Example 1

Can you imagine the interview? "What data structure would you use to store 1 to 10 values of the same type?" — "10 variables?" — "Correct!"

Yes, that is horrible. But ...

1. Does it work?

   However clumsy this solution to problem is, if the system is running and has been for a while, one must assume that it does.

2. Is it clear what it does and how it does it?

   Again, it is a clumsy way of doing it, but it is pretty obvious how it does it.

   Programmers may be offended by it, but they should not have any trouble understanding it.

   And from the business persective it is working code contributing to the revenue stream.

   And users will be completely oblivious to the horrors.

3. Is there a history of errors requiring fixes for this piece of code?

   I don't have access to the modification history; but cursory inspection of the style and layout suggest that this was probably all written by the same author at the same time.

   It is possible that extra layers have been added, to accommodate expansion of the permission types etc.; and that could hold a clue as to how it ended up like this.

   Perhaps it started out as just one or two levels deep and was essentially okay; but because of the style used for the original implementation -- the use of what appear to be globally scoped scalars to carry state -- the simplest way to extend the permissions was to stick with the existing mechanism and just add levels of nesting; because to do otherwise would incur knock on changes affecting huge amounts of the codebase.

4. Beyond the generic -- better code is just better -- is there anything specific that would be improved by refactoring this piece of code?
5. How many other pieces of code would require changing in order to allow this piece of code to be cleaned up?

   As offensive as the use of global state may be to modern programmers used to languages that provide for modularisation; limited scoping and data hiding; there are thousands of programs in other languages -- COBOL & Fortran to name but two -- still running, that use that style to process huge amounts of data every second of every day.

   As offended as you might be by them; those programs were often 'state of the art' when they were first constructed. And they don't stop working because ideas about program construction have changed.

   Unless you can find some measurable, concrete benefit to the bottom line to justify changing the code -- and changes here will inevitably require big changes in many other pieces of code -- that adds up to a huge risk for no measurable reward.

   That is to say, if you implemented the change to put 10 values into an array, made the required changes every where else in the code base that would be affected and got it right first time; what you end up with is a working system that is fractionally cleaner; but will not improve the customer experience of bring in more revenue.

   Unless you can find that measurable benefit from the improvement; the risks far outweigh the benefit of "cleaner code".

Example 2

Essentially, exactly the same as above.

Whilst not pretty, it is pretty simple and clear.

Unless there are improvements that are entirely localised to that subroutine, which given the number of external (presumably global) variables being referenced and changed, would probably be very superficial, then you have to look at the affect of any changes here on the rest of the code base and do the risk .v. reward calculations.

Which on cursory inspection do not look good.

---

I suspect that the only way to tackle refactoring this codebase would be to start by converting the global state into a god object and passing that around to every subroutine. Once you've done that, you can start breaking the god object into sub-objects that only get used by subsets of the functions; and then slowly work them into separate modules or classes.

I've done that a few times and it is a pretty thankless task. In the interim you break things a lot; and even once you've got it all working again, it often runs measurably more slowly and often requires more memory, and all you've done is get back to where you started in terms of functionality.

Of course, the hope is that future additions and maintenance are easier; but that only becomes measurable in the future.

Get it wrong and you're the guy that broke everything.