In the past week or so I’ve been going over all my code with the new guy who’s going to replace my in my current job(Yeah, I’m about to leave, but that’s a whole different post).
It's strange. I have done code reviews in the past, but never had the pleasure of going through *all* my old code, class by class, line by line almost, with someone with a completely different perspective and experience than me. It was enlightening.
I found that:
· No matter how well you think you might code, you’ll always be able to find a sloppy line or implementation somewhere. We all make those “mistakes”. I quoted that word because usually there’s a pretty good reason why these things were written as they were. There usually is no good reason for keeping them that way (not including special occasions), but somehow, because we’re all human, the code looks just like that – human.
I discovered a whole slew of OOP inconsistencies, starting with naming guidelines, up to standard method names not being descriptive enough, and even some un-optimized loops and stuff.
· It’s always great to get a fresh perspective on things. I found several things I could have done better, and several things that I did influenced the new guy on stuff he didn’t know. It was great to get some validation on my design decisions.
· You can never have too much documentation. I found this out the hard way. My company never seemed to find the time and budget to document the code using a 3rp part tool such as Document!-X or the likes, so what I ended up with was a half baked set of documents and code comments. I twasa sad tail. Everything has to be explained “manually”. No real documents exist to tell the story(although I tried to give as much as possible using all sorts of long comments and a few word documents). Although all the design documents were there, it really wasn;t what was expected, and I feel it could have been a lot better. The end result is I had to struggle to find the right place to even start explaining how our infrastructure works, and an even bigger battle explaining stuff that even I didn’t remember their purpose.
· Self-documenting code is the way to go. I cannot stress this enough. I’ve found, going through 1 year old codebase, that I can easily remember and understand what each method and class does just because the names were meaningful enough to “get” it without trying too hard. Even the new guy got some of these things without me having to explain it to him. The stuff I did have to explain was usually either very complicated and logical, or written poorly enough to not be understood the first time around. The rule of thumb here is “If you need to write a comment on that method it is either too long, or not written in a self documenting way”.
· You should always document your database tables and their reason for being. This is another biggie. We have a database that has plenty of “probably” obsolete tables in it that we cannot delete because some legacy C++ code might be using them. The C++ team does not want to take any risks so everything just stays as it is. Sometimes the table design is pretty horrible, and without good documentation you can be left wondering why that “ID” column is of type “nVarChar” instead of an IDENTITY field. Luckily, I’ve kept a neat and ordered word document specifying each tables purpose in life, its relevant tables relations, a description of each columns purpose and all the columns that are not used. Of course I also listed “currently unused” tables in this document as well. This helped a lot when explaining the database’s structure to the new guy, since I suspect no one in our company really knows for sure exactly what each table does. It like a “father to son” sort of fairy tale information passed using only stories, and no books written about it. A real myth. Had I not created that word document, we’d be in real trouble today.