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.