.NetASP.NETC#Code reviewDevTweetTwitterVB.Net

Highway to hell? DevTweet: @mariannerd and @aahoogendoorn review code

The following conversation between Marianne and me will be published as our DevTweet column of SDN Magazine, issue 102.

clip_image001[52]

Listening to Black Sabbath’s Master of Reality.

clip_image002[42]

@aahoogendoorn Look at you… new avatar! Is that an AC/DC shirt?

clip_image001[53]

@mariannerd Howdy partner! It is AC/DC all right. Your avatar new too? Something wrong with the old one?

clip_image002[43]

@aahoogendoorn Nope, just a change in scenery! Like the project I’m doing. I’m asked to do a code review.

clip_image001[54]

@mariannerd Cool! Whenever I’m asked to do a code review, the client always knows, or at least suspects, something’s terribly wrong.

clip_image002[44]

@aahoogendoorn Oh yes… We have the suspect… Now it’s a case of getting the evidence … I feel like Columbo..

clip_image002[45]

@aahoogendoorn You know the suspects from the beginning, but you still have to look for the evidence for conviction!

clip_image002[46]

@aahoogendoorn So when you are asked to do a code review, what is the first thing you do?

clip_image001[55]

@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?

clip_image001[56]

@mariannerd Next, I make a list of all things I should look at during the review. Architecture, domain, data access, layers, extensibility..

clip_image001[57]

@mariannerd Security, authorization, reuse, code copying (especially in VB), way of working, documentation, design, quality of coding.

clip_image002[47]

@aahoogendoorn So far so good!

clip_image001[58]

@mariannerd Rule #1 in code reviews: more time means more detail.

clip_image002[48]

@aahoogendoorn Code Reviews are fun! No deadlines. just looking at somebody else’s code.

clip_image002[49]

@aahoogendoorn … Oh and telling what’s wrong with it… I must say this is the first time I get such an assignment.

clip_image001[59]

@mariannerd. You know, there’s so much horrible code out there, we could do code reviews for the rest of our lives.

clip_image002[50]

@aahoogendoorn Well a lot of this stuff is subject to opinion… 100 developers, 100 different code for the same functionality…

clip_image001[60]

@mariannerd Yes true, but some code is definitely worse than other code. And believe me, I’ve seen some bad coding in my time.

clip_image002[51]

@aahoogendoorn Any examples?

clip_image001[61]

@mariannerd Examples of bad code? How many do you need. Check out this old blog post of mine (in Dutch). http://htxt.it/reVB

clip_image001[62]

@mariannerd. Think of SQL statement in web pages, or of a single class that handles 50% of all functionality.

clip_image001[63]

@mariannerd. Or think of the same business rule implemented multiple times on different locations, in different ways.

clip_image002[52]

@aahoogendoorn One of the things I came across: internal web application: authentication by doing a LDAP query to see if the user exists.

clip_image002[53]

@aahoogendoorn Haven’t they heard of Windows Authentication in IIS? They didn’t even check if the user was enabled or disabled… :-S

clip_image001[64]

@mariannerd Haha, nice one. I once audited the enterprise web portal for a very large international company where ALL communication …

clip_image001[65]

@mariannerd between front end and back end went through one single class. Imagine the effect of a single change..

clip_image001[66]

@mariannerd Sometimes, it gets really bad. Code a whole company depends on, which is so bad, that productivity will definitely be below zero soon

clip_image001[67]

@mariannerd Anyway, I love being asked for code reviews. It gives the opportunity to help improve the quality of applications.

clip_image001[68]

@mariannerd. That is, if they follow up on your advice from your code review. Unfortunately, that is not always the case.

clip_image002[54]

@aahoogendoorn Well maybe it’s not needed… Or they just want to know the risks and are happy to take them!

clip_image002[55]

@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.

clip_image001[69]

@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.

clip_image001[70]

@mariannerd. But I just love clean code…

clip_image002[56]

@aahoogendoorn …. But even bad code can function.

clip_image001[71]

@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!

clip_image001[72]

@mariannerd. As a consequence, of the over 40 pages in the application, only 6 compiled (the ones they visited running it in the browser)…

clip_image002[57]

@aahoogendoorn LOL "we got a live one here!". This business application compiled at runtime? No test cases just put it in production!

clip_image001[73]

@mariannerd. And then things can get pretty messy. Especially when your poor company lives of this software.

clip_image002[58]

@aahoogendoorn That is the whole point isn’t it.. Developers just start coding without a plan, or knowing best practices.

clip_image001[74]

@mariannerd Yes, a lot of developers code without having a decent architecture, without patterns, without layers – but with best intentions.

clip_image002[59]

@aahoogendoorn Oh yes, best intentions… The road to hell is paved with good intentions…

clip_image001[75]

@mariannerd. No stop signs, speed limits. Nobody’s gonna slow me down. Highway to hell. AC/DC# ?

clip_image002[60]

@aahoogendoorn Nope… 4 minutes by Madonna LOL.

clip_image001[76]

@mariannerd. Madonna? Girl you need a music review instead of a code review!

clip_image002[61]

@aahoogendoorn ….I guess even bad music can function. 😉