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 (http://thc.org/root/phun/unmaintain.html)
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! ;)
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.
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.
Quote from: "Cova"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".
Quote from: "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.
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.
Quote from: "Cova"Quote from: "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.
Quote from: "Thorin"
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.
Oh, oops, forgot that:
8. Document, Document, Document! :P
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++);"
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.
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.
for($TemporaryLoopingVariable=0;$TemporaryLoopingVariable<count($ArrayOfPeople);$TemporaryLoopingVariable++){} vs.
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.
Quote from: "Thorin"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.
Quote from: "Shayne"for($TemporaryLoopingVariable=0;$TemporaryLoopingVariable<count($ArrayOfPeople);$TemporaryLoopingVariable++){} vs.
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.
Quote from: "Shayne"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.
Quote from: "Shayne"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.
Quote from: "Cova"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...
That's very true, optimised code is useless if it doesn't work ;)
Incedentally, naming local scope variables isn't usually a big deal unless you do something like this:
/* Nest this a few more times and the single letter counter gets confusing */
for (int i = 0; i < blah.length(); i++)
{
for (int c = 0; c < blah[c].length(); c++)
{
//TODO: ADD CODE HERE
}
}
Which again, illustrates the point that good coding standards are nearly always discretionary.
Ya - besides gratuitous use of i, j, and k as index variables for looping, I also often use X and Temp as temp places to store garbage, and when using C usually also a 'char buffer[big]' for assembling strings in before sending them off to places. If following standards is good for maintainability, and i is the standard indexing variable - well using i must be maintainable then.
The one thing I have started doing a lot of lately, thats greatly improved my ability to figure out my own code after I've forgotton some/most of it, is commenting my if/else code blocks (I normally don't comment a lot). This helps the most when the code blocks start nesting 5+ levels deep, with else's way down below somewhere, and a page or more long when it gets hard to see what block/scope you're in. I wish VB had a good way to graphically indicate the matching start/end of code blocks. I usually start by blocking-out the sections, then fill in the code to deal with 'em later. So I'll start with (vb-like psuedo-code ahead)
If File Opened Successfully Then 'Deal with contents of file
Else 'Deal with error opening file
End If 'End of dealing with the file
Which becomes something like..
If File Opened Successfully Then 'Deal with contents of file
If File Is Empty Then 'Deal with empty file
Else 'Process contents
End If 'End of empty file check
Else 'Deal with error opening file
Select Case on error.
Case 1 'comment desc of error 1.
...
End Select 'end list of error codes
End If 'End of dealing with the file
Eventually, when the app is near-complete, somewhere you end up with something like
End If
End If
End If
End If
End If
End If
Or a series of closing curly-braces, language don't matter much. Anyways, when you've realized there's a bug and need to add a bit of code to the end of a block, or add an else-block somewhere, it's much easier to figure out where to put it if you don't have to scroll up 10 pages trying to match indentation-levels. And since I typically write all of the if/else/end-if statements first, even if I don't know what I'll need I include all the else's, I rarely ever have bugs due to un-closed code blocks, and I just comment-out empty blocks at the end if I didn't need 'em.
Wow! Some really fantastic posts here. I defenitely like the point of good coding standards being discretionary, it's really true. Sometimes you can have assy syntax but if your tabulation is good and your mistakes are as regular as a swiss watch there's a chance that whoever has to work on your code in the future can sort out what needs to be sorted out. If everything's akimbo and it's hard to distinguish where things start and stop, but things are put in you-know-it-works chunks like:
// begin area code chunk
// Copy and paste this chunk of code to extract the area code from the phone number!
if eval(isNumeric(right(strip(phNum,'/,(,),-,[,]'),3)), someVar) {sendAC(saveAC(someVar));} else {throwCodeError(451);}
// end area code chunkThen I find that not so bad. It's a chunk, it's re-useable! ...Just as long as it's clear what the I/O of the chunk is.
Out of the whole list I liked this the best:
QuoteRigidly follow the guidelines about no goto, no early returns, and no labelled breaks especially when you can increase the if/else nesting depth by at least 5 levels.
This reminds me of what I call break-dancing. Can the break just go to some error handling or some kind of section of the code that's closer to the end? NO!!!!!! It MUST go to the TOP and the MIDDLE and EVERYWHERE freaking INBETWEEN making tracing the actual steps of a common execution almost impossible. Nothing gets me making the evil eye at people faster than something like (excuse the computerlish demi-syntax):
setVarbs: set variable1 = defaultValue1;
code...
code...
/*there's a secret break in this function that kicks out of the current function being executed! SNEAKY!*/
itsARecursiveStatement: someMysteryFunction(CONST_PIE);
code...
if (somestatement XOR 0) {
goto setVarbs;
}
else
{
goto itsARecursiveStatement;
}
onError goto setVarbs;
GRAGHGH! Hate so much. I consider break-dancing to be wiggity-wack. Or at least the hallmark of those who can't manage to cram their processes into a logical flow.
Cova, you just provided an excellent example of where a bit of commenting makes code much easier to understand/edit.
As for VB6 code like you've got there, back when I wrote it I used to always move blocks of code into their own Subs as much as possible. If a Sub or Procedure had more than two pages of code, it was too hard to read. C# and VB.NET have the #region/#endregion directives that make it *way* easier to hide blocks of code that you don't normally look at.
And Mr. A., you make an excellent point. You don't necessarily make code more maintainable by following strict de rigueur rules; case in point - Hungarian Notation. It's all about making it as understandable as possible.
Well, when I talk about VB, I don't distinguish really whether it's .NET, VB6, etc. I've been using VB since version 3 (it was 16-bit back then, I remember VB4 added 32-bit) and the syntax really hasn't changed since then - they've just added more keywords and extra functionality. The snippets I posted above could just as easily be .NET as VB3, you'd just have system.io.file objects to the the file stuff in .NET, and OPEN and PRINT statements in VB3. I don't really use the #region statements in .NET much - I think I've done it once since .NET 1.0 was originally released. I prefer to make my code readable without depending on a specific IDE. I do use visual studio to code, intellisense is the best thing ever for lazy coders like me, but I also very commonly have a few notepads open with various code files from other projects / samples - I don't like to open any file in visual studio that isn't part of the current project/solution, and loading multiple instances of visual studio eats RAM in a serious way (though I do do it sometimes)
Hmm..., maybe I see things differently because I'm mostly a VB programmer now. Especially my own code, due to a few standards I tend to follow (not NAIT's standards), tends to look very englishy. My variable naming for instance - I usually prefix most variables with an a (sometimes an an) if its an instance of an object - so I end up with aDatabaseConnection, aCommand, aResult, etc. sometimes shrunk down for easy typing of commonly used ones (aDBCon is common). Throw in some other VB code which is already more english-looking than most languages, and you get stuff that even non-coders can probably read, even without comments. 'If aDatabaseConnection.Open = True Then aCommand.Execute()' is a perfectly valid piece of code, assuming those 2 variables exist.
As for logical flow of code - I think it might be a thing of the past. Way-back in the early days, code was spaghetti - goto's everywhere. Then came functions, procedural programming, etc. - and code flowed very obviously from main() to whever it ran out of lines to execute. Then we went object-oriented, and code-flow started to get a little confusing again - you might have multiple-inheritance, overloaded functions, etc., you have to be careful and know what you're calling, where it is, make sure you aren't calling an inherited version of it (or that you are getting the inherited one if you're dealing with the base object), etc. Code still flows, but it's harder to follow. Windows added messaging and event queues and such to that - windows programs respond to mouse-click events, raise-exceptions, and do all kinds of things that can really result in execution jumping all over the place in a program. Event-based programs don't flow anymore from start to finish - they process messages and every time they need to check to make sure the pre-req's for that message have been met (eg. print message is useless if no doc is open yet). And finally, we are just on the verge of something new again - multi-threaded programming. Yes, there have been multi-threaded programs around for a long time, but with multi-core cpu's, hyperthreading, all next-gen consoles going multi-core, etc. the average programmer is now starting to need to learn about parallel threads of execution - multiple flows of code potentially modifying the same memory, and all responding to events with execution jumping all over. A single copy of an object that's been inherited 10 times being accessed by 20 threads at once - modern code needs to be concerned with doing its own part of the whole properly and error-free, and not so much anymore with flow.
Quote from: "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! ;)
"To foil the maintenance programmer, you have to..." And thus my interest is re-piqued (being, technically, the "maintenance programmer" for
RTW when I started here 21 months ago) -- I am totally re-reading this at home, it's been prolly 2 years since I last found this list. :D
PS: agreed re. Java.
PPS: "Quidquid latine dictum sit, altum sonatur."
- Whatever is said in Latin sounds profound.
^ I had this as my BBS sig in the pre-Internet days, no joking :o
"This essay has been like rock candy, seed the string with sugar, soak in sugar water, soon it grew out of control"
-- the original author (!)
http://mindprod.com/jgloss/unmain.html <-- (essay is split into sections)