Should I try to convince my client to fire his current developers?
The code clearly exhibits that the developers do not understand programming in several instances. There are is-a relationships used where has-a would be appropriate as well as code that is just completely in the wrong freaking place, to put it bluntly. Misspellings are abundant and there is no documentation. All the code is formatted like trash and I seriously doubt that even the original developers (an outsourcing company who are obviously specializing in making shitty websites as fast as they possibly can and didn't even bother to open up a programming book before trying) would be able to read the code.
Any advice on explaining this to him? None of the code is in version control, documented, or has any heirarchical structure. Tons of code is repeated, there is voodoo code, yoyo code, bulldozer code, and several examples of developers copy pasting and obviously not reading docs at all.
Here is one of the cutest things I found:
function formatAvgRating($average_rating){ 124 if($average_rating <= "0.25" ){ 125 $average_rating = "0"; 126 }else if( $average_rating > "0.25" && $average_rating <= "0.75" ){ 127 $average_rating = ".5"; 128 }else if( $average_rating > "0.75" && $average_rating <= "1.25" ){ 129 $average_rating = "1"; 130 }else if($average_rating > "1.25" && $average_rating <= "1.75" ){ 131 $average_rating = "1.5"; 132 }else if($average_rating > "1.75" && $average_rating <= "2.25" ){ 133 $average_rating = "2"; 134 }else if($average_rating > "2.25" && $average_rating <= "2.75" ){ 135 $average_rating = "2.5"; 136 }else if($average_rating > "2.75" && $average_rating <= "3.25" ){ 137 $average_rating = "3"; 138 }else if($average_rating > "3.25" && $average_rating <= "3.75" ){ 139 $average_rating = "3.5"; 140 }else if($average_rating > "3.75" && $average_rating <= "4.25" ){ 141 $average_rating = "4"; 142 }else if($average_rating > "4.25" && $average_rating <= "4.75" ){ 143 $average_rating = "4.5"; 144 }else if($average_rating > "4.75" ){ 145 $average_rating = "5"; 146 }
Should I tell him this is how a competent individual would write it?
function formatAvgRating($rating) { $rating_floor = floor($rating); $part = $rating - $rating_floor; if ($part < .25) return $rating_floor; else if ($part > .75) return $rating_floor + 1; else return $rating_floor + .5; }
???
So many decisions..
15 comments
[ 3.2 ms ] story [ 52.2 ms ] thread1) Do your contract. If asked then respond with that.
2) Do a good job, let your client see clearly YOUR code is superior.
3) Let your client keep their shitty coders.
By stating the above, you potentially destroy a relationship with a client. Unwarranted too.
If the replacement isn't as cheap as the apparently horrendously long code, can you show noticeable differences in runtimes?
It takes some mental energy to actually look carefully at the cleaner solution.
Different strokes...
The example code is exactly the type of thing that would raise my hackles - only a really, really bad developer would write that. It's not only easier to write it correctly, but faster.
Let me challenge that and modify it just a bit, and we'll be in agreement: "only a really, really bad developer would write that and actually think that it was a good, elegant implementation."
Set aside this specific case, and we can look at the more general state of things...
See, there are other contexts where there are different priorities. I have worked in environments where a large if-block was preferred to an elegant solution. This typically happens if you are extending a big enterprise app, have lots of junior developers on a team, or expecting to hand code off to a developer of questionable skills. Thus the business, and everyone in the decision tree, would be happy with the former implementation. If you happen to get TOO fancy, you might even get tagged as someone who writes unmaintainable, obfuscated code that only you can understand. Seen it! Gotta love politics in the office, eh?
I've also worked in an environment where there was social pressure to write really slick code. It probably wasn't the same level of pressure that startups seem to apply to themselves, but you could be sure to raise eyebrows (and heckles) in code reviews if you used something like the former 'cute' example. Architectural controls reigned those in, and people learned quickly.
In the end, I would say that you can't look at one friggin' function definition and proclaim someone to be a 'bad developer'. Maybe they built the conditional logic from the wording in a functional spec on a late Friday afternoon when they were zoned out and they intended to refactor the code later.
I haven't had the pleasure of experiencing an environment where a large, hard-coded IF block (or whatever) was preferred to a more stable, maintainable solution, so I can't comment on that.
But one of my clients has a brand new codebase written by an "expert" contractor that has all but killed the business. I call it "startup fail" and it's sad to see that stuff happen.
Especially when I saw it coming and warned the owner not to hire this particular guy. I suppose it's made me a little knee-jerky when I see code that he would almost certainly have written and been proud of.
If you're a consultant, hired to help them improve the product and their development process, than you can advise on how the company may train or otherwise improve their situation with the existing staff.
If you're a contractor, hired to write some specific features for a project and nothing more, than suggesting what they do with their staff is way beyond your scope of interest and will just likely get you canned.
Even if you say yes to the above I say don't do it because karma is a bitch. But I doubt you will say yes to the above.