The following conversation between Marianne and me will be published as our DevTweet column of SDN Magazine, issue 102.
|
Listening to Black Sabbath’s Master of Reality. |
|
|
@aahoogendoorn Look at you… new avatar! Is that an AC/DC shirt? |
|
|
@mariannerd Howdy partner! It is AC/DC all right. Your avatar new too? Something wrong with the old one? |
|
|
@aahoogendoorn Nope, just a change in scenery! Like the project I’m doing. I’m asked to do a code review. |
|
|
@mariannerd Cool! Whenever I’m asked to do a code review, the client always knows, or at least suspects, something’s terribly wrong. |
|
|
@aahoogendoorn Oh yes… We have the suspect… Now it’s a case of getting the evidence … I feel like Columbo.. |
|
|
@aahoogendoorn You know the suspects from the beginning, but you still have to look for the evidence for conviction! |
|
|
@aahoogendoorn So when you are asked to do a code review, what is the first thing you do? |
|
|
@mariannerd The first thing I do? Ask the client what they’re hoping to achieve: do they actually want to know how deep the rabbit hole goes? |
|
|
@mariannerd Next, I make a list of all things I should look at during the review. Architecture, domain, data access, layers, extensibility.. |
|
|
@mariannerd Security, authorization, reuse, code copying (especially in VB), way of working, documentation, design, quality of coding. |
|
|
@aahoogendoorn So far so good! |
|
|
@mariannerd Rule #1 in code reviews: more time means more detail. |
|
|
@aahoogendoorn Code Reviews are fun! No deadlines. just looking at somebody else’s code. |
|
|
@aahoogendoorn … Oh and telling what’s wrong with it… I must say this is the first time I get such an assignment. |
|
|
@mariannerd. You know, there’s so much horrible code out there, we could do code reviews for the rest of our lives. |
|
|
@aahoogendoorn Well a lot of this stuff is subject to opinion… 100 developers, 100 different code for the same functionality… |
|
|
@mariannerd Yes true, but some code is definitely worse than other code. And believe me, I’ve seen some bad coding in my time. |
|
|
@aahoogendoorn Any examples? |
|
|
@mariannerd Examples of bad code? How many do you need. Check out this old blog post of mine (in Dutch). http://htxt.it/reVB |
|
|
@mariannerd. Think of SQL statement in web pages, or of a single class that handles 50% of all functionality. |
|
|
@mariannerd. Or think of the same business rule implemented multiple times on different locations, in different ways. |
|
|
@aahoogendoorn One of the things I came across: internal web application: authentication by doing a LDAP query to see if the user exists. |
|
|
@aahoogendoorn Haven’t they heard of Windows Authentication in IIS? They didn’t even check if the user was enabled or disabled… :-S |
|
|
@mariannerd Haha, nice one. I once audited the enterprise web portal for a very large international company where ALL communication … |
|
|
@mariannerd between front end and back end went through one single class. Imagine the effect of a single change.. |
|
|
@mariannerd Sometimes, it gets really bad. Code a whole company depends on, which is so bad, that productivity will definitely be below zero soon |
|
|
@mariannerd Anyway, I love being asked for code reviews. It gives the opportunity to help improve the quality of applications. |
|
|
@mariannerd. That is, if they follow up on your advice from your code review. Unfortunately, that is not always the case. |
|
|
@aahoogendoorn Well maybe it’s not needed… Or they just want to know the risks and are happy to take them! |
|
|
@aahoogendoorn Which could be the outcome of a code review. It’s not good, but far from worse.. And it may be fixed with a couple of quick wins. |
|
|
@mariannerd. Well yes, that could be the outcome. Maybe I’m to idealistic about writing good code. If it ain’t broken, don’t fix it. |
|
|
@mariannerd. But I just love clean code… |
|
|
@aahoogendoorn …. But even bad code can function. |
|
|
@mariannerd My favorite example: an ASP.NET web site that was never compiled by the developers. They just ran it from the browser. Great stuff! |
|
|
@mariannerd. As a consequence, of the over 40 pages in the application, only 6 compiled (the ones they visited running it in the browser)… |
|
|
@aahoogendoorn LOL "we got a live one here!". This business application compiled at runtime? No test cases just put it in production! |
|
|
@mariannerd. And then things can get pretty messy. Especially when your poor company lives of this software. |
|
|
@aahoogendoorn That is the whole point isn’t it.. Developers just start coding without a plan, or knowing best practices. |
|
|
@mariannerd Yes, a lot of developers code without having a decent architecture, without patterns, without layers – but with best intentions. |
|
|
@aahoogendoorn Oh yes, best intentions… The road to hell is paved with good intentions… |
|
|
@mariannerd. No stop signs, speed limits. Nobody’s gonna slow me down. Highway to hell. AC/DC# ? |
|
|
@aahoogendoorn Nope… 4 minutes by Madonna LOL. |
|
|
@mariannerd. Madonna? Girl you need a music review instead of a code review! |
|
|
@aahoogendoorn ….I guess even bad music can function. |
![clip_image001[52] clip_image001[52]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00152.jpg)
![clip_image002[42] clip_image002[42]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00242.jpg)
![clip_image001[53] clip_image001[53]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00153.jpg)
![clip_image002[43] clip_image002[43]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00243.jpg)
![clip_image001[54] clip_image001[54]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00154.jpg)
![clip_image002[44] clip_image002[44]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00244.jpg)
![clip_image002[45] clip_image002[45]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00245.jpg)
![clip_image002[46] clip_image002[46]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00246.jpg)
![clip_image001[55] clip_image001[55]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00155.jpg)
![clip_image001[56] clip_image001[56]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00156.jpg)
![clip_image001[57] clip_image001[57]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00157.jpg)
![clip_image002[47] clip_image002[47]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00247.jpg)
![clip_image001[58] clip_image001[58]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00158.jpg)
![clip_image002[48] clip_image002[48]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00248.jpg)
![clip_image002[49] clip_image002[49]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00249.jpg)
![clip_image001[59] clip_image001[59]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00159.jpg)
![clip_image002[50] clip_image002[50]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00250.jpg)
![clip_image001[60] clip_image001[60]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00160.jpg)
![clip_image002[51] clip_image002[51]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00251.jpg)
![clip_image001[61] clip_image001[61]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00161.jpg)
![clip_image001[62] clip_image001[62]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00162.jpg)
![clip_image001[63] clip_image001[63]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00163.jpg)
![clip_image002[52] clip_image002[52]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00252.jpg)
![clip_image002[53] clip_image002[53]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00253.jpg)
![clip_image001[64] clip_image001[64]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00164.jpg)
![clip_image001[65] clip_image001[65]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00165.jpg)
![clip_image001[66] clip_image001[66]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00166.jpg)
![clip_image001[67] clip_image001[67]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00167.jpg)
![clip_image001[68] clip_image001[68]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00168.jpg)
![clip_image002[54] clip_image002[54]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00254.jpg)
![clip_image002[55] clip_image002[55]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00255.jpg)
![clip_image001[69] clip_image001[69]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00169.jpg)
![clip_image001[70] clip_image001[70]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00170.jpg)
![clip_image002[56] clip_image002[56]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00256.jpg)
![clip_image001[71] clip_image001[71]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00171.jpg)
![clip_image001[72] clip_image001[72]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00172.jpg)
![clip_image002[57] clip_image002[57]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00257.jpg)
![clip_image001[73] clip_image001[73]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00173.jpg)
![clip_image002[58] clip_image002[58]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00258.jpg)
![clip_image001[74] clip_image001[74]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00174.jpg)
![clip_image002[59] clip_image002[59]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00259.jpg)
![clip_image001[75] clip_image001[75]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00175.jpg)
![clip_image002[60] clip_image002[60]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00260.jpg)
![clip_image001[76] clip_image001[76]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00176.jpg)
![clip_image002[61] clip_image002[61]](http://sanderhoogendoorn.com/blog/wp-content/clip_image00261.jpg)