How to write unmaintainable code!

[Tonnica]

I wouldn't be surprised if some of you have already seen this but I found it again while searching for some ASM hacking tools and got a good chuckle.



How To Write Unmaintainable Code



Apologies to anyone non-coding inclined who decided to cick on the link and need to take an asprin after trying to scan it over. Double apologies for those who are coding inclined and get the Scanners Effect from considering what kind of code these guidelines would produce (WOO! BOOM!). Triple apologies to those who have coded in Java. Ever. I feel your pain!

[Cova]

I just scanned through it, only read a couple that looked interesting.  It's amazing how many of these just happen to show up at NAIT for various reasons - some in my code and some in other places.



That entire section on testing for instance - like any of that happens at NAIT.  We test basic functionality (mostly) - no fancy use cases, no performance testing, etc.



I'll also give a couple examples of things I've written reasonably recently that no-one else at NAIT can seem to wrap their heads around.  The first one is a pretty simple little DLL used by most of the web site - all  it does is figure out where you are on the site (based on your URL), decide which menu you should have on the left side (all menu's for everything stored in a db), and build a little tree-like structure of it in memory along with tagging the menu item for the current page so it can be highlighted and such, and send all that off to some javascript someone else wrote that renders it nicely and gives it effects and such.  Besides various helper functions for eg. querying the db and such, it's just 1 recursive function, and the database is a single table that has a many-to-many relationship with itself.



And my new favorite is a tool I'm still working on to monitor performance stats from all my servers, cause I couldn't find anything free already available that I liked.  It's highly multi-threaded, with many small threads that don't do much besides wake up occasionally to ask a server for some perf data, and other threads that make sure I'm monitoring the right servers and other things related to the health of the program.  Basically, the entire program is based on doing some very small part of something, and assuming that some other thread will take care of everything else eventually.  And before I get too many suggestions on open-source monitoring software (there is a LOT of it out there), I am using RRDTool already from this thing that I'm writing to do the vast majority of the work.

[Thorin]

Being a maintenance programmer yet again, I've vowed never to deliberately write new code in such a way that no one can understand it.

things I've written reasonably recently that no-one else at NAIT can seem to wrap their heads around ... a simple little DLL used by most of the web site ... build a little tree-like structure of [the menu] in memory

I suppose the menu structure is maintained by hand in the database?  The way you describe is the way I would've done it two years ago.  Now, I'd suggest making a menu.xml file to read, that way there's no database traffic.  Plus, the intrinsic structure in an xml file just *screams* "hierarchical menu data".

[Cova]

Being a maintenance programmer yet again, I've vowed never to deliberately write new code in such a way that no one can understand it.

I write code thats efficient - that usually has the effect of making it rather hard for others to understand.



As for the menu thing - one of the requirements given to me before I wrote that was that the menu definitions were all stored in a db - I usually do use XML files for storing small bits of data like that.  In this case though, the db works quite well.  Considering that there are many menu's for many apps on different servers, with some apps load-balanced across multiple servers that must be kept in sync, and development servers that need their own version of the menu's and also an easy way to migrate menu's from dev into production.  In SQL we just have 2 menu db's (one prod, one dev), it's easy to move things between them with a couple SQL statements, and it's easy to back up the db's, restore them, etc.  Actual editing of the menu's is done through a web-based app that a different person on my team wrote - the editing of menu's is integrated right into the portal software now.  Also, my little component uses extensive caching to keep db trips to a minimum - considering everything it does it actually performs very well, much better than the consultants we had at the time thought it would, they told us that to meet all the requirements such a component would never be fast enough for production.

[Thorin]

Being a maintenance programmer yet again, I've vowed never to deliberately write new code in such a way that no one can understand it.

I write code thats efficient - that usually has the effect of making it rather hard for others to understand.

Ugh.  Sorry, but easy-to-understand does not automatically indicate a loss in processing efficiency.  In fact, code that is hard to understand is much harder to performance-test because it's difficult to understand what to test.  And if you're not running proper performance tests, you *cannot* claim that your code is efficient, because you have no way of proving it.



Now, the things that make code easier to understand can generally be summed up as:



1. Comments that describe why code does something

2. Comments that explain complex algorithms right where they're being calculated

3. Comments that tell you what other code or help file to look at or gives URLs for the complex bits being performed

4. Using variable names (without acronyms) that describe the data they hold

5. Being anal about proper spelling all through the code base

6. Thinking about the structure and flow of the code

7. Naming methods (Subs/Procedures/etc) so they describe what they do



None of those steps has any impact on efficiency as I define it: the speed at which an application performs its work.



If you really don't agree with those statements, then I hope to hell that I never have to maintain your code.

[Melbosa]

Now, the things that make code easier to understand can generally be summed up as:



1. Comments that describe why code does something

2. Comments that explain complex algorithms right where they're being calculated

3. Comments that tell you what other code or help file to look at or gives URLs for the complex bits being performed

4. Using variable names (without acronyms) that describe the data they hold

5. Being anal about proper spelling all through the code base

6. Thinking about the structure and flow of the code

7. Naming methods (Subs/Procedures/etc) so they describe what they do

Not that I disagree with any of these, but you sound like my DB Admin here at work, almost word for word.  All your missing is her "Document, Document, Document" statements.

[Thorin]

Oh, oops, forgot that:



8. Document, Document, Document!

[Cova]

I don't disagree with anything you said Thorin - that list is all things you can do to make code easier to read/maintain.  However, code that is highly efficient/optimized is typically harder to maintain than code written to be maintainable.  Reasons for this fall all over the place, but include things such as language selection, architecture of the application, what algorithms are used, etc.  VB code is normally pretty easy to read/maintain, C is harder, and assembly is damn tough - but for maximum optimization of the little loops that use up most of the cpu time, it's still not uncommon to find a bit of hand-optimized assembly around.



Also, while code that is very well documented is likely also easy to maintain (just follow the instructions), I wouldn't necissarily call it maintainable.  Ya - a person could hack some changes together and it would probably work.  But to actually maintain something and be responsible for it, I like to understand why it was written the way it was - get a lot deeper than 99% of documentation goes.



As for perf testing, I did do a lot of load testing on that menu app, and I do some other load testing on other things I write if they will be used in production and are accessed often.  But, likely due to the lack of testing, we do often encounter performance problems and I'm the guy that normally has to fix them.  I have kind of a unique perspective on performance here (also since I'm the guy that does server capacity planning and such).



And because it just popped into my head, I'm going to post up my favorite little code optimization, which I would classify as unmaintainable.  Granted, with a few pages of documentation most people could figure it out, but if you need that much documentation to describe 1/4 of a line of code, well I still consider it unmaintainable (and would use it if I ever needed to).  This little snippet copy's a string in C (ok, char-array) - it basically re-implements the strcpy function:  "while (*s1++ = *s2++);"

[Thorin]

I think we have two different opinions on what is considered maintainable code.  From your last post there, it seems that you think that genius-level code is not maintainable.  Lets take the snippet you posted:



while (*s1++ = *s2++);



Now I remember enough about C that I know this loop will continue until an assignment from s2's data can no longer be made to s1's data.  Now, looking at the code I have no idea *why* we want to copy this string.  It's completely out of context.  Lets assume we're copying a password, 'k?  And that we're using this different technique because it's faster.  So how about this:



// faster than strcpy

while (*copyOfPassword++ = *originalPassword++);



That's the kind of difference I'm looking for.  First a quick comment as to why we're using this code instead of the standard strcpy() function, and then variable names that tell me (without hunting through the rest of the code) what kind of data should be in them.



I don't think that genius-level code should be maintained by junior-grade developers.  Clearly, the maintainer's skill level has to be commensurate with the code he or she is maintaining.  But even genius-level hand-optimized assembly benefits greatly when its creator passes on in one way or another why he or she did it the way they did.  Code that cannot be understood frequently gets tossed even if it's absolutely miraculous.

[Mr. Analog]

In my experience, unless genius-code works 100% of the time it will be scrapped eventually.



Maintainable code means flexible code, if it can't be altered because it has been ?beroptimised then over time it will die off like the cancer it is. Personally I've found that design patterns are more powerful than clever code. It's possible to write sophisticated code and plug it into a framework to make everyone's life easier, if the flexibility is planned for. In older systems it's usually regarded as easier to slap on a band-aid fix, like dropping an optimised chunk of code into a prexisting codebase, but taking a phased approach by refactoring that codebase over time has more benefits to the overall application as maintenance continues.



So, using your example, we could wrap your strcopy replacement into a new static string class wrapped in a utility DLL that you phase into code by first adding it to places that require optimization, but later roll it application wide.

[Shayne]

Code Select Expand

for($TemporaryLoopingVariable=0;$TemporaryLoopingVariable<count($ArrayOfPeople);$TemporaryLoopingVariable++){} vs.

Code Select Expand

for($i=0;$i<count($people);$i++){}


I dunno, my code isnt very maintainable from somebody else looking at it, but i know my coding practices so i know what im looking at.  Though i do find it easy enough to adapt my code to the look of others.



I have pet pieves though. Like skipping the use of curly brackets.  Billion objects to do what a single one could do.



Maintainable i think is in the eye of the beholder.  What i find maintainable could be drastically different to what a amature coder things is.

[Cova]

Now I remember enough about C that I know this loop will continue until an assignment from s2's data can no longer be made to s1's data.  Now, looking at the code I have no idea *why* we want to copy this string.

Not quite - in C there's no concept of unable to assign data from one place to another.  C will quite happily allow you to write anything you want all over the systems RAM, which will of course quickly result in either the entire system crashing, or Windows killing the process (the first is more likely on older windows versions, newer versions are better with their memory protection and process isolation).  This is why we have things like buffer-overflow attacks, of which there is no protection for in the code above (not that strcpy has any either though, use strncpy for that).  The above code uses several techniques that most people aren't familiar with, a few are listed in the original link on unmaintainable programming, and all are things I tend to like to do.



First off - realize that 0 is false, anything else is true, and in C your conditional statements can be based on any variable type at all.  This is important because its why the while loop ends - C strings are null-terminated, so while copying byte-by-byte when it copies the null at the end the return value of the assignment is also null, so the loop stops.  In C its common to have just a single variable (or function return value) as the condition for an if (or loop, whatever) perhaps with a not in front, as you quite often need to check for non-0 return values, null pointers, etc.



Next bit of tricky code, and where a lot of people get confused, but one of my favorite techniques when coding in C/C++ - pointer arithmetic.  All those ++ operators in that snippet are actually incrementing memory addresses, which is why the while loop actually moves from the first character to the end of the string.  This is also one of the things listed on the originally linked article, apparently the order of operations for the ++ operator is undefined and some C compilers would have issues with this code - this is quite a widely used example for what you can do with pointers and such though, and I have yet to see a compiler break it.



As for actually using that code in an application, I don't think I ever would.  strcpy is probably faster.  If you haven't allocated memory yet, strlen + malloc + memcpy would be faster and you'd be safe from buffer-overruns.



Really, it depends what you're building as to whether highly optimized or maintainable code is the better solution.  For most business apps, maintainability is more important and more hardware to make it run fast is cheap.  Usually various RAD tools are used for software like this (Java, .NET, etc.)  We also all call this bloat-ware, but usually only when produced by a company we don't like (eg. MS).  On the other hand, something like a core game engine is likely highly optimized C/C++, with some hand-written assembly to speed up a few places where a profiler told you your code was slow.



As for my clip being "genius level" - actually I first saw that clip in my first-year C++ class in CST here at NAIT.  And I've used that clip a few times since while teaching other people how pointers work - due to its short length its just an excellent piece of code for that, while having almost no real-world usefulness.  We likely do have differing opinions on what "maintainable" code is - but my point of view isn't "genius level" - its understanding WHY a piece of code is the way it is, and a short comment that just says "faster than ____ function" only tells me why the code is there - not why its faster/better, why its important that its fast instead of easy to read, etc.  Kinda like the difference between knowledge and wisdom.

[Thorin]

Code Select Expand

for($TemporaryLoopingVariable=0;$TemporaryLoopingVariable<count($ArrayOfPeople);$TemporaryLoopingVariable++){} vs.

Code Select Expand

for($i=0;$i<count($people);$i++){}

I'd actually prefer to read the second one.  Using i and n are *so* commonplace these days, and its use in the for loop *clearly* indicates its only being used as a counter.  The condition is also very clear - without thinking about it, I know you're looping until you reach the end of "$people", whether that's a structure or an array or whatever.

I have pet pieves though. Like skipping the use of curly brackets.  Billion objects to do what a single one could do.

Yah, ditto.  Another one'd be methods/Subs/functions with huge parameter lists.

Maintainable i think is in the eye of the beholder.  What i find maintainable could be drastically different to what a amature coder things is.

I do believe you hit the nail right on the head there.  Perhaps I should change my statement to:



Being a maintenance programmer yet again, I've vowed never to deliberately write new code in such a way that someone with my level of knowledge cannot understand it.

[Thorin]

Also, while code that is very well documented is likely also easy to maintain (just follow the instructions), I wouldn't necissarily call it maintainable.  Ya - a person could hack some changes together and it would probably work.  But to actually maintain something and be responsible for it, I like to understand why it was written the way it was - get a lot deeper than 99% of documentation goes.

Just so you see I'm not just trying to pick a fight here, I completely agree with this statement.  However, making code that's easier to maintain (as per the seven things I mentioned) will help you get into the code faster and help minimize your errors when changing it.  Especially when it's code you wrote two or three years ago and you can't remember every single detail about it...

[Mr. Analog]

That's very true, optimised code is useless if it doesn't work