KISS Principle: Cleaning Up Code Left by Previous Programmer

[Borg]

For the last week, I have been cleaning up code left by a previous programmer who clearly didn't know what the KISS principle was. I removed over 75 class files that were completely unnecessary and made the code ridiculously difficult for others to trace through. Most of this was code that could have been put in a single method but instead, he would create several interfaces, implementing classes, unnecessary Spring wiring and enums for as few as two potential options - all while mixing front-end and back-end processes such as enums with database implementations embedded. When you got to what was actually doing the work, it was the method that should have been written in the first place without all of the extra fluff. After all, why KISS your code when you can take it out back and violate it completely?


I'm wondering how many programmers have had to deal with this kind of nonsense. Do you run into code left by previous developers where they created stuff that was way more complex than it needed to be? I would be interested to hear your stories.

 

[anorlunda]

Can it be that the previous guy worked in a shop where lines of code (LOC) was the metric of productivity?

 

[Borg]

Can it be that the previous guy worked in a shop where lines of code (LOC) was the metric of productivity?

It's the same office I work and that isn't considered. I have seen that though. My managers know that my LOC when I'm working with existing code is usually negative. My previous record was a 10,000 line class file that I reduced to 2000 lines for a -8000 LOC. I'm probably somewhere well past -100K over the last year with this project.

 

[anorlunda]

That reminds me of the time when I consulted on Y2K remediation. Big companies had huge piles of code that nobody remembered what it did our whether it was necessary. However there was no motivation to find out if it was really needed. Y2K provided the motivation.

Unfortunately, there is no metric that measured how much cleanup happened globally in those years, but it was a lot. One anecdote points the way. Prior to Y2K, the disaster recovery systems of most companies was an untested rats nest of s***. But on 9/11/2001 the offsite backup centers of those companies housed in the WTC worked flawlessly.

The public image of Y2K is that of silly wasted efforts. But I believe that the benefits were enormous, but alas not measured.

 

[anorlunda]

@Borg, your post about negative LOC stimulated my neurons. I can't resist telling one more old war story.

I consulted for a big company during Y2K. They had 375000 PCs. Many of their employees, when faced with a PC upgrade, backed up the old PC and restored those files on the new PC. That included who knows what executables from who knows what sources. For example, old Word Perfect documents, or Dan Bricklin Visicalc spreadsheets that potentially were used for mission critical purposes. As a result, their PCs had 4-5 generations of old software installed, but no documentation on what was actually used.

They could have done 375,000 employee interviews to find out which files were needed for what purpose. They could have done 50 million (my estimate) forensic investigations of all those files for Y2K compliance. Instead, they elected on one weekend to wipe the hard disks on 375000 PCs and to install a standard image on each. Of course, they had 375000 angry employees on Monday who lost their personal files, their email archives, and their favorite spreadsheets. (Suppose someone did that to your laptop this Saturday. How angry would you be?) The hit job had to be done stealthily because we feared that if was announced in advance, that many employees would back up their disk on Friday and restore it on Monday.

Nobody could say with certainty what disruption the hit job might have caused for the company's business. It took great courage to make that decision. The end result, was that by Tuesday, nothing big happened and that ugly history was rapidly forgotten. It was a huge lesson on the difference between what's possible and what's probable.

So that does not challenge your negative LOC record @Borg, because we couldn't count the LOCs. But I think it makes a fun story.

 

[jedishrfu]

While you might not have a KISS policy, your manager may be secretly evaluating you on your checkins and this programmer was keeping your { clueless | trusting } manager happy.

Another source of problems can come from new programmers who learned about java classes and interface design. Hence for every class you create an interface in case that class needs to be "replaced" with a new implementation that is plug compatible. Interfaces can make things better but they also make things hard to track through at the source level instead you need a runnable program and a good debugger.

(I always dreamed of a kind of source code debugger that tracked data through your code just by clicking on the variable of interest. Interfaces would require the programmer to select an implementing class in that case)

For every great programming paradigm there is some programmer ready to make a mockery of it.

 

[Borg]

Count me in the group that backs up data like that. My company has been aggressively limiting software installations on their computers which is a pain for developers because we have to install and test software as part of our job. It's gotten better but, for a while installed software would disappear a few days after I installed it. This led to some interesting debug-fests trying to figure out why everything suddenly stopped working. Fortunately I have an administrative account now that limits the aggression on their part.

 

[jedishrfu]

Variety breeds chaos but conformity opens the door to disaster. (dangerous hack attacks).

Where all men think alike, few men think at all and that's why we need women managers.

 

[Borg]

While you might not have a KISS policy, your manager may be secretly evaluating you on your checkins and this programmer was keeping your { clueless | trusting } manager happy.

I don't make any secrets about the amount of code that I've been removing and everyone on the project knows what a coding mess we're dealing with. My last two yearly performance appraisals were rated outstanding so, if she's evaluating me on that, she's happy about it.
After checking in one particular rewrite last year, I asked the other developers on the team to let me know if they notice anything different in case I broke something. One guy said right away that he noticed something. He said that the server no longer took 5 full minutes to start and was now down to less than a minute.

 

[jedishrfu]

Even negative LOC are checkins so she knows you're doing your job.

There also used to be a programmer sentiment of making things as convoluted as possible to keep your job.

What ever happened to the programmer that wrote the awful code ?

Another thing I've noticed is that when faced with a complex application or problem and a short deadline to fix something invariably folks will go for the surgical approach with local changes only to minimize breaking other stuff.

Sometimes this means making a new class or a duplicate of a new class so as not to break the old class.

 

[Borg]

Even negative LOC are checkins so she knows you're doing your job.

True. I'm just thankful that my paycheck isn't proportional to my LOC because I wouldn't get paid much.

 

[Borg]

There also used to be a programmer sentiment of making things as convoluted as possible to keep your job.

The two self-serving phrases that I hate to hear are "job security" and "that will look good on my resume". Neither of them serves the customer.

What ever happened to the programmer that wrote the awful code?

He apparently didn't feel like maintaining the code that he created and moved on to another project. There were also 60 property files and a ton of manual processes that had to be done by admins on the production server. I've eliminated most of those as well.

Another thing I've noticed is that when faced with a complex application or problem and a short deadline to fix something invariably folks will go for the surgical approach with local changes only to minimize breaking other stuff.

Sometimes this means making a new class or a duplicate of a new class so as not to break the old class.

There has been a good amount of that going on and I've been repairing that as well. Some of it was my own from when I first started on the project and didn't have the depth of knowledge about it as I do now.

 

[newjerseyrunner]

Most likely scenario for me would be future-proofing. I often have a solution that I know will work, but purposely chose to make it slightly more complicated so that it can be generic for whatever changes happen in the future. I like being able to simply pull a class out, drop a new one in, and have the functionality be completely different.

I also found myself thinking that when I was a young programmer. The concept of business logic talking to a database seemed most efficient and easy to work with. I ended up with a bunch of MySQL calls in high level logic. Then came the day. "one of our clients want to use Oracle instead of MySQL, that should be easy right?" "uhhhhhhhhhhhhh" I've come across the same thing in graphics, people will purposefully abstract things because they don't know if there final product will have OpenGL or DirectX behind the scenes. Interfaces end up being particularly far removed from the window API logic. Your code should know what a window and a mouse click is. It should not care if it came from GFLW or WinAPI or X.

 

[phinds]

Although I've had some bad experiences w/ incompetent previous programmers, I've not had any as bad as yours. Close maybe. What I HAVE had, many times, is enormous problems with code being VERY hard to maintain because of complete or effectively complete lack of proper documentation. Programmers love to program but most of them hate to document. I was often an unpopular manager because of my insistence on extensive documentation except in those cases where the programmers knew that THEY were going to have to maintain their code. Only novice programmers think that in a few years they will have a clear understanding of code just because they were the ones who wrote it.

 

[Borg]

Then came the day. "one of our clients want to use Oracle instead of MySQL, that should be easy right?" "uhhhhhhhhhhhhh" I've come across the same thing in graphics, people will purposefully abstract things because they don't know if there final product will have OpenGL or DirectX behind the scenes. Interfaces end up being particularly far removed from the window API logic. Your code should know what a window and a mouse click is. It should not care if it came from GFLW or WinAPI or X.

Exactly. One of the reasons that I had to do some of this was that the customer wanted encrypted connections to the database. Unfortunately, they used a Spring-Mongo implementation that did not support encryption. Upgrading the Spring-Mongo wasn't an option because it would have had to been updated to a version that would have broken all of the Spring code. So, I had to hunt down and replace all of the data connections to use the DAL that I created last year. That DAL was upgraded by just setting the mongo driver to a new version. My DAL layer is build so that I could switch the entire application to use a different database without much effort.

 

[jedishrfu]

I always went with documents in source code and even had a script to extract them out to make a document. This was in the days of Bookmaster and C/C++ coding. The comment blocks had style similar to Java code blocks. I would generate an overview doc by extracting the first comment block of each file and a reference doc by extracting all comment blocks of a certain style patterned after C standard function descriptions:

/** method : one liner describing the method / comamnd
*
*   This method/module does the following...
*
** syntax:
*
** returns:
*
** notes:
*
* ...
*
**/

The logic was: Its hard to blame a programmer for not updating the documents but if they changed the code and not the comment then it's "I've got you and your little dog Toto." time.

 

[Mark44]

I always went with documents in source code and even had a script to extract them out to make a document. This was in the days of Bookmaster and C/C++ coding. The comment blocks had style similar to Java code blocks. I would generate an overview doc by extracting the first comment block of each file and a reference doc by extracting all comment blocks of a certain style patterned after C standard function descriptions:

/** method : one liner describing the method / comamnd
*
*   This method/module does the following...
*
** syntax:
*
** returns:
*
** notes:
*
* ...
*
**/

For a method or function that has parameters, there should also be a brief description of each parameter, including the permissible values.

If a method can throw exceptions, these should be documented, as well.

The logic was: Its hard to blame a programmer for not updating the documents but if they changed the code and not the comment then it's "I've got you and your little dog Toto." time.

Having comments that are out of phase with the code is probably worse than not having any comments at all.

 

[phinds]

If a method can throw exceptions, these should be documented, as well.

But not JUST documented. I've often found cases where programmers did not handle possible error conditions because they didn't they the would ever be thrown. Programmers don't always make good testers.

 

[jedishrfu]

We had some very unique testers at work. One report we got was a race condition that if you did something in some sequence it would fail. The programmer had fixed it twice before and each time the tester reproduced it until finally no matter how fast the tester hit those keys he couldn't get it to fail ... and then it did. A sad day for that programmer losing once again to the fiendish tester.

Another time, I had a tester on my team that found a small api issue with some example code. He wrote a defect and the programmer closed it as trivial. He wrote it again and again adding more content and more force each time until the programming team conceded that it was a design issue that they didn't know how to fix for this release.

Yet another time, I had a tester, during system test, write up a defect that the datawarehousing software component was breaking her MP3 player. Of course, we're thinking what? that"s not part of our system test plan. Eventually though we tracked it down to a memory issue and as thanks the company laid her off as part of an annual corporate downsizing (no good deed goes unpunished).

 

[newjerseyrunner]

I prefer the nice standard DOxygen format simply because there are boatloads of parsers that'll both generate it for you, and convert it to wiki pages.

/**
@function foobar
@accessibility private
@desc This makes a foo and does bar on it
@param foo_t foo - the foo to bar
**/

 

[jedishrfu]

To be clear my document format was circa 1980s and I modeled it after the lattice C manuals we had. I used Rexx to process the files and collect the comments into a Bookmaster and a few years later a Framemaker document.

It was during the time of stone knives and bearskins but I digress...

 

[kroni]

Kiss principle is not good because efficiecy imply complexity. Just look at the evolution of cars, before you have a simple motor, four wheel, now you have sensors, injector, optimal control, data fusion. To get efficiency the complexity need to be augmented.

The thing i learn by working with other people is : We are not always right, sometimes other people make thing for apparently no reason but good reason.
May be all the thing are not useless no ?

 

[newjerseyrunner]

Another possibility is code rot. If I replace a piece of code, I may or may not remove the original file from my code base. While I try my best to keep my repository clean, I know that there will always be at least a few files that no longer even get compiled in. For example, I have a file here called stringUtils.h. As soon as I started working here, I replaced the string class, so all of those functions are no longer used... except a function called now() which returns a timestamp in a char array. I had no equivalent in my string library, so I left it for now while I tackle bigger issues. But I didn't remove the hundred other functions that are in that header; I know that the compiler will remove them for me.

 

[QuantumQuest]

For the last week, I have been cleaning up code left by a previous programmer who clearly didn't know what the KISS principle was. I removed over 75 class files that were completely unnecessary and made the code ridiculously difficult for others to trace through. Most of this was code that could have been put in a single method but instead, he would create several interfaces, implementing classes, unnecessary Spring wiring and enums for as few as two potential options - all while mixing front-end and back-end processes such as enums with database implementations embedded. When you got to what was actually doing the work, it was the method that should have been written in the first place without all of the extra fluff. After all, why KISS your code when you can take it out back and violate it completely?


I'm wondering how many programmers have had to deal with this kind of nonsense. Do you run into code left by previous developers where they created stuff that was way more complex than it needed to be? I would be interested to hear your stories.

I've encountered similar issues in the past, with most notable one that I was asked to be almost the whole team(!) for a java mobile system by a small company, when Java Mobile was almost in its infancy and with strict deadlines(!), using a rudimentary code base that had almost to be rewritten all over itself(!). A second case was writing some extension APIs and tools in PHP. The codebase was written by a programmer involved in various projects. The first thing I noticed by some simple inspection, was the complete lack of comments and documentation and a programmer that persistently "was using his right hand to scratch his left ear", as we say in my country. It was not about trying to be more abstract for future proof code. It was just an attempt to retain his position, by effectively prohibiting other programmers to work with his code. The previous programmer in the position that I got was really an excellent one and the only thing he said was that he had told all this I did, but for very strange reasons, regarding company management, to no avail. So, after wearing the hat of the magician for a while and developing some ad hoc rabbits, I was completely convinced not to get involved to such messy things ever again. For me - as for every developer I think that respects him / her self, carefully written code, comments and documentation are not some theoretical concepts needed only when you are at the university. I cannot think of anything better than what Donald Knuth said about writing code:

“The best programs are written so that computing machines can perform them quickly and so that human beings can understand them clearly. A programmer is ideally an essayist who works with traditional aesthetic and literary forms as well as mathematical concepts, to communicate the way that an algorithm works and to convince a reader that the results will be correct.”

Unfortunately, even in these days there are people that still think that one has to confuse / disallow other people to work on their code.

As for the KISS principle, I think that it's meaning and its application not only means many things to different people but also spans many levels, regarding many factors / parameters and I'm mostly talking about the software development world. For me, its essential meaning is absolutely correct. If someone develops a GUI then this meaning is more easily manifested, for what the user should have to interact with. On the other extreme, it may potentially mean - and it vastly does in practice, a different level of meaning for the development of code for the GUI. Even using libraries / frameworks / template systems, the complexity is there. It's just hiding. As complexity of the requirements for an application, or a software system in general, increases, so does the level of KISS with Simple getting more complex. But in my opinion KISS is still there, waiting to award the one who finds it. From a different perspective, there is no absolute meaning of KISS among different developers with different skills. It begins like common sense for a small project but as the software grows larger and more complex the gap increases, getting eventually to the question "how simple is simple?". So, putting aside the common sense meaning in some cases, KISS is like an ever expanding tree.

 

[Borg]

Kiss principle is not good because efficiecy imply complexity. Just look at the evolution of cars, before you have a simple motor, four wheel, now you have sensors, injector, optimal control, data fusion. To get efficiency the complexity need to be augmented.

The thing i learn by working with other people is : We are not always right, sometimes other people make thing for apparently no reason but good reason.
May be all the thing are not useless no ?

Using your car analogy, would you route the gas line through the passenger compartment and put a valve and Wifi on it just in case the user wanted to turn off the gas manually or using a smart phone? Or, route three different gas lines - just in case... And finally, just for fun, send all of the user messages to the displays in Chinese or Arabic depending on the day of the week and translate them back to English just before the user sees them.

I'm not against complexity where its needed, I am against complexity that serves no purpose. Working with complex code isn't the problem that I'm talking about, it's the complete overkill of building 10 unnecessary things when one will do only one is needed. When a programmer builds something overly complex and doesn't document anything, they are purposely trying to frustrate other developers whether they want to admit it or not.

 

[Vanadium 50]

uilding 10 unnecessary things when one will do.

Do you mean when only one unnecessary thing will do?

 

[Borg]

Do you mean when only one unnecessary thing will do?

Yes, that.

 

[ChrisVer]

Do you run into code left by previous developers where they created stuff that was way more complex than it needed to be?

I am not a programmer, but to be honest I don't think that the codes I deal with are overcomplicated. Or at least I don't think they are- whatever I don't understand is because of my weaknesses.
The main problems I deal with are:
1. weird names, that someone came up because he found them funny at that time- and maybe they were- but later on when someone joins out of the blue you don't expect for him to know the origin of the joke... on the contrary I'm left with an impression "what is that?" and spend a few hours trying to understand what it is.
2. DOCUMENTATION MISSING. That is the worst and it is very usual in the codes I deal with. For real, with a good documentation, even a not so simple code can be: (a) put to run immediately and (b) partially modified or developed. When instead you get a huge code, with several classes etc, and there is not a single comment line explaining what their purpose is, huh I just want to kill the one who wrote it.
There are codes that took me a month or so just to be able to use them, and codes that took me a few minutes. What was the difference between the two? a few extra lines between /* (or ''').

Now, for piece of codes I write daily for my work- I am pretty sure that they are not the best in the world... in fact a lot of them are the worst possible, because I write them on a whim just to do my job fast. So for example when I finished my master, there was a small free (and of course non-vacational) period of ~a month, when I sat down and decreased the size of the code I used for my analysis to the 1/10th of the original. Now the size keeps on growing exponentially until next leisure time...
But hey I don't distribute my sourcecodes- and if I ever will I am going to spend some time cleaning up the mess and of course providing some guidelines.

 

[jedishrfu]

Nowadays the only time you gets docs are when the API becomes a public entity and a lot of folks want to know how to use it otherwise it's just javadocs if even that. I think the reason for the docs is it's less time consuming than answering the same questions over and over. It's so much easier to say go read the xxx docs and look at the examples.

Naming variables is an art which some programmers do poorly. My old boss at GE would name his variables as nuts1 and nuts2 for every macro assembler program he wrote. His view was the name gets compiled away anyway so why waste time coming up with a clever name.

For me though I prefer the clever name, and sometimes will some variant of symonyi notation to group variables by type.

 

[newjerseyrunner]

I find proper variable naming to be important.

m_bIsInvisible - is a member Boolean and tells whether an object is visible. You need to know exactly nothing about the code to know that.

p_csName - c string function parameter indicating name.

I hate see foo, bar, x, I... all over the place.

 

[Borg]

I agree with the naming conventions. Often on projects, when I am trying to track down how something works, I will invariably run into multiple, unrelated classes with identically named methods like processData(). Performing a search on stuff like that is impossible when there are a lot of them. IDEs are pretty good at figuring out usages except for cases like the obfuscated code that I'm having to deal with. It has been painful. It shouldn't take lead develpers a full day to figure out how to change the displayed text on a web page but it often does. If the guy had at least not used so many common names that were constantly changing as they traversed the code...

 

[Mark44]

The best exposition of variable names I've seen is Steve McConnell's "Code Complete." Chapter 9 of this book has suggestions for how a programmer should name the variables being used, together with examples of useful names and awful names.

Here's a Coding Horror example:

X = X - XX;
XXX = Aretha + SalesTax( Aretha);
X = X + LateFee( X1, X) + XXX;
X = X + Interest( X1, X );

I've seen a lot of Fortran code with names just as bad as those in this example.

As McConnell says, "the most important consideration in naming a variable is that the name fully and accurately describe the entity the variable represents."

He talks about the Hungarian convention for variable names, and cites advantages as well as disadvantages of this naming scheme.

 

[Borg]

The Naming section in the link in my signature has some great examples of poor naming. Number 18 has always been my favorite:

18. Bedazzling Names
Choose variable names with irrelevant emotional connotation, e. g.:
marypoppins = ( superman + starship ) / god;
This confuses the reader because they have difficulty disassociating the emotional connotations of the words from the logic they’re trying to think about.

 

[TurtleMeister]

A technique that I often use with Visual Studio is to define my variables in nested structures or classes. This helps me a lot with my poor memory because the IDE will show me a drop down of all the members of the struct or class as I type out the variable name. An example would be: TestData.Mux1.Voltage(x) or TestData.Mux2.Amps(x) When I type TestData. the IDE will show me a dropdown of all instruments connected, and when I continue by typing TestData.Mux2. the IDE will show me a dropdown of all data arrays for that instrument. The question for me recently has been whether to use structs or classes. From what I've read, struct variables are stored on the stack and class variables are stored on the heap. So the struct variables should be faster but the class variables should be more memory efficient.

 

[ChrisVer]

and what is Mux1 or Mux2? multipliers? However I belong to the people who use vi [at least in office- at home I have IDEs], and this feature is not available to me...
I plan to move soon.

 

[TurtleMeister]

and what is Mux1 or Mux2? multipliers? However I belong to the people who use vi [at least in office- at home I have IDEs], and this feature is not available to me...
I plan to move soon.

Yes, it's short for multiplexer.

 

[Borg]

I still am amazed at the stuff that I continue to find. In one process that processes XML for storage into the database, I found the following:
XML sections are processed via a file that describes the class to use for a particular field.
One set of entries changed had an additonal config setting that changed the name of the field slightly.
The class file (ActualImplClass below) then has enums that define those particular name changes and the code looks for those instead of the original name like everything else.
Additionally, the ActualImplClass of course implements an interface with ActualImplClass being the only implementation of that interface in the system.
The ActualImplClass is also never called directly. Instead, there is another class which provides it as a variable.
That variable is also obfuscated by instantiating it from a string variable that never changes and then instantiates it through a class loader - something like this:

public class FinalClass extends ImplementationClass{
    private String implClassName;

    public FinalClass(){
        implClassName = "com. biz.ActualImplClass";
        // ActualImplClass implements SomeInterface and is the only implementation in the entire project (very typical)
    }

    public SomeInterface instantiateInterface(){
        Class<SomeInterface> interface = (Class<SomeInterface>)getClass().getClassLoader().loadClass(implClassName);
        return interface;
    }
}

implClassName also has a setter and getter so that you could change it on the fly but nothing ever called it.
What all of this basically does is change a name unnecessarily and bury that fact in a bunch of obfuscated code that was completely unnecessary.

 

[SlowThinker]

The question for me recently has been whether to use structs or classes. From what I've read, struct variables are stored on the stack and class variables are stored on the heap. So the struct variables should be faster but the class variables should be more memory efficient.

Sorry for being a bit late to the party, but I'm pretty sure that, in C++, the only difference between struct and class is that the members of struct are public by default, and members of class are private by default. So, for example,

#if MARYPOPPINS
struct Superman {
#else
class Superman { public:
#endif

should do the same thing, whether MARYPOPPINS is true or false.

 

[Mark44]

From what I've read, struct variables are stored on the stack and class variables are stored on the heap.

No, that's not true. A variable of either type that is declared inside a function is allocated space on the stack. Variables that are declared using the new operator are allocated on the heap. However, I suppose it would be possible to use the "placement new" operator to return a pointer to previously allocated memory, possibly on the stack. You would have to go to extra effort to do this.

So the struct variables should be faster but the class variables should be more memory efficient.

As far as I know, neither of these is true. The stack and the heap are just memory -- they just happen to be in different sections of it. Access to either part would take the same amount of time, and a variable stored on the heap uses just as much space as one stored on the stack.

 

[TurtleMeister]

Thanks, I had read conflicting information on programming forums and sites about the stack vs heap in regards to speed. When googling "which is faster heap or stack", the overwhelming consensus seemed to be that the stack is faster. But it is the internet, and sometimes sites will just copy the same wrong information from each other. Some seem to think that the difference is not really worth considering. But sometimes I like to play around with n-body simulations where I need to access a lot of variables with as much speed as possible. So I'm thinking it might make a difference in that application. But I'm getting off topic so I'll leave it at that. Thanks for the replies.

 

[Svein]

I was often an unpopular manager because of my insistence on extensive documentation except in those cases where the programmers knew that THEY were going to have to maintain their code. Only novice programmers think that in a few years they will have a clear understanding of code just because they were the ones who wrote it.

Hear, hear! When I was in charge of program development,I always insisted on having a clear program description before I allowed anybody to start programming. When that was done, the description was imported into the program as a sequence of comments and the actual code could be stuck in between the appropriate comments.