The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Rediculous Code Review?

What do you do when your resident senior developer / code reviewer always demands you follow advice counter to every experienced developer who has ever lived?  When your appeals to reason are responded to with the intellectual equivalent of "because purple bicycle upside down cake, that's why"?
anon
Saturday, December 06, 2008
 
 
Either

1) check in my ego at the door and just do what he wants

2) quit and find a new job

3) quit and not find a new job so that I don't have to follow anybody's standards I disagree with. Not a good option though!
Steve McLeod Send private email
Saturday, December 06, 2008
 
 
What Steve said.

But also, try real hard to understand why he might be right.  A lot of people who seem like total fools at first glance actually know something we are resisting learning.  I'm not holding out a lot of hope, here.  It's just that you can't discard this possibility out of hand.

Do you have peers?  What do they think of Mr. Purple Bicycle?  What does his supervisor think of him?
Walter Mitty Send private email
Saturday, December 06, 2008
 
 
So there's no architect here? Nor an appropriately technical manager? This senior is the top of the food chain, bar none?

First, take some deep breaths. Yes, it can suck, but hey, people clash all the time. We all have our opinions and differing experiences. What's the worst outcome of sucking it up and just doing what the senior says, at least for now? Typically poor architecture/coding practices will evidence themselves sooner or later.

Second, "every experienced developer who's ever lived" ... or just you, since you can't speak for the rest of us? Are the practices truly WRONG, or just different than what you're used to? Is this technology or platform relatively new to you? How many years does the senior have with the tech, and with the company? Are they otherwise deserving of their position, or did they somehow luck into it?
Andrew Badera Send private email
Saturday, December 06, 2008
 
 
ps: "ridiculous" not "rediculous"
Andrew Badera Send private email
Saturday, December 06, 2008
 
 
"every experienced developer who has ever lived"

Be careful with your appeal to authority. Your resident senior developer / code reviewer just might be right sometimes.
dev1
Saturday, December 06, 2008
 
 
What do I do?

Argue, way too often.


What *should* I do?

Decide there are more important things in life than winning this argument, and just change it the way he wants.  And update my resume.
Kyralessa Send private email
Saturday, December 06, 2008
 
 
Seek enlightenment ... for you or them :-)

If what is being suggested is truly counter to accepted custom and practice then you should be able to find plenty of examples of documented standards with reasoned arguments which support your position.

Then sometime, in an informal, non thraatening non confrontational way respectfully ask about the reasons for diverging from the published approaches.

Sometimes there really are goods reasons for doing things in what may seem to be odd ways. [Speaking to myself here] Be wary of assuming that the other guy is an idiot.
Dave Artus Send private email
Saturday, December 06, 2008
 
 
Did you get paid for the code you wrote?  Then you don't own it anymore.  Do it the way they want, even if it's the stupidest thing you've ever heard of.

And if you want to afterwards, update the resume.

But I guarantee that the next job will have something they want done that is equally stupid.

After a while you learn that all shops are dysfunctional, just in slightly different ways.  The only way out is to start your own firm, and then you can enforce your dysfunction on someone else.
:)
xampl
Saturday, December 06, 2008
 
 
Thanks for the advice.  For the record, the code review advice includes such gems as "copy and paste this 30 lines worth of code all over, for maintainability".

Saturday, December 06, 2008
 
 
Quick Point.

Ensure that put a comment in each location you paste the code so you can find it later so when your paid for refactoring tool points out the obvious.

Include date time and the person ordering the change or you will cop it.
John Griffiths
Sunday, December 07, 2008
 
 
> For the record, the code review advice includes such
> gems as "copy and paste this 30 lines worth of code
> all over, for maintainability".

With so little detail to go on I am guessing....

But if the feedback from the reviewer was to undo some of your code refactoring, then the reviewer was 100% correct.

When it comes to large systems, if it ain’t broke, don't fix it.
Jussi Jumppanen Send private email
Sunday, December 07, 2008
 
 
First of all - if you're going to argue it's not enough to assume you know better. But...

Step 1: Ask yourself if the argument is worth winning because there are often political costs to this fight. Nobody likes a complainer and if the rest of the team follows the same standard and you're being a contrarian, you can be as right as rain and still be very wrong. There may be reasons you're not privy to. As a lead myself, I've had to tell some junior folks - "Yes, you're totally right... but just do it wrong, please. We can't afford the risk at this point.

Step 2: Amass evidence from less questionable sources than yourself of better approaches and come at your senior like, "Hey whatyaknow?! Learn something new every day... it looks like there may be a better way - check this out..." You may be surprised, sometimes what you thought you knew is wrong for reasons you never considered. It sounds much better to be 'contributing' than, "Dude, you are SO wrong and I am vindicated." Ego matters. It wouldn't if we were all robots, but we alas, we aren't.

This puts your senior in a position to yell at your for contributing (for which he would likely get his own wrists slapped in any sane organization) or to tell you explicitly 'thanks, but not thanks - please just do you work.' At which point, I'd advise you to suck it up and take solace in the fact that you know better, even if you can't enforce it (yet).

Monday, December 08, 2008
 
 
(A) Maybe you are naive or don't have enough software development experience...
(B) You have enough software development experience, but your senior supervisor sees you as unexperienced, just because you're younger than him, and doesn't matter what you do, he always will look at you as naive...
(C) Sometimes (A), sometimes (B)

Suggestion: Try to have an open mind, & learn new things from others, thats for (A). If that doesnt work (B), you may want to look for a job elsewhere.
mrk Send private email
Monday, December 08, 2008
 
 
I would follow his advice, and explain what I think is a better way.  A discussion is always useful, maybe you don’t quite understand his points, or he doesn’t understand yours. You may even earn a lot of respect out of the whole ordeal.

Personally I think coding standards should be dictated by the language, but allot of people develop them for a company.  But hey, any standard is better the no standards.
Jon
Tuesday, December 09, 2008
 
 
Maybe you like the rest of your job and want to stick around. Suggest an improvement to the review procedure to your line manager.

Often, code reviews are one on one.  An approach that worked well for me previously was to conduct the review as a team.  Have at least one peer along with the senior in the room.  Both with prepared comments on your code.  A team discussion of the motivation behind such a comment usually ends up on a positive note.
Draco Send private email
Tuesday, December 09, 2008
 
 
To give an example of where you think the technical reviewer is an idiot, when in fact he might not be...

I worked in a place that had a big, nasty VB .NET code base.  It was ripe for refactoring, and I did a lot of it.  And in most cases the code was indisputably clearer when I'd finished.

The trouble was, the code was branched.  So while I was merrily refactoring version 1.1, another developer was spending a lot of his time merging version 1.1 with version 1.0-with-bug-fixes.  It's *very* hard to merge when every line of a file has changed, and you can't easily see what the substantive differences are.

So while I was theoretically right that the code needed refactoring, there were outside constraints that made refactoring a bad idea.
Kyralessa Send private email
Tuesday, December 09, 2008
 
 
Sounds like ever Millennial programmer I have ever worked with.  Personally I am sick of new programmers thinking they know everything, because at a previous job they did "X" and you do "Y" and "X" is better.  I don't care, I have made these programmer rewrite an entire modules because they do not listen, they eventually quite, and move on to another company because the grass is greener... and now with the downturn in the economy, several of these Millennial programmer have been laid-off and have called me back to ask for their job back and guess what... no job here.
anon
Tuesday, December 09, 2008
 
 
I have worked at many places with absurd rules and standards. Usually there is some obscure reason, often enough an absurd or wrong one.

Here is the deal. You are working for them. They are paying you. They tell you what to do and you do it.

Following dumb coding standards is within the scope of your job description. If they want you to drive a forklift or unload 100 lb sacks from a truck, you can rightly refuse as it is not part of your job.

Or perhaps you accepted the job at less pay than you are worth to work on interesting problems and they did bait and switch and now your are doing pointless busy work. It's valid to say "I am willing to do busy work, but that's not what I signed on for, so to do it I need a $30/hr raise."

Also, if they make you spend time on busy work like this and then tell you to work unpaid overtime to get the REAL work done, you can legitimately refuse.

But if you are working 8 hrs a day and they want to tell you to do dumb things, do them, then cash your paycheck and enjoy life at home after 5pm.
Tony Chang
Friday, December 12, 2008
 
 
Things to think about:

1) How do you know you are right?

2) Just because some author claims his pattern is standard or best doesn't make it so.
    a.  Always analyze patterns.
    b.  Always compare various implementations of paterns
        with the same name.

3) Many times it is best to write something so it is functional, readable and maintainable FIRST and then go back and optimize later the areas hit the most.

4) In the real world "get it done" counts for a whole lot more than get it done perfectly.
S
Monday, December 15, 2008
 
 
In a software department with more than a couple of enginneers there needs to be some kind of standardisation with regards code layout and formatting, naming conventions, data types and a list of do's and dont's. I was the purple bicycle and my motto was 'it may not be the right way but it's the only way here'

So adapt or leave is my advice, mavericks are not good in any software department.
Large One Send private email
Monday, December 22, 2008
 
 
The code inspection part is easily fixed for you personally, let someone you trust give you a second opinion, then write the code the way your boss wants it, this time with your head cool and sane. And then try to achieve enlightenment by reading Joel instead:

"But I reserved another hour every afternoon before going home to improving the process. I used that time to fix things that made it hard to debug our product. I set up a daily build and a bug database. I fixed all the longstanding annoyances that made development difficult. I wrote specs for the work that I was doing during the day. I wrote a document explaining step-by-step how to create a development machine from scratch. I thoroughly documented an important internal language which had been undocumented. Slowly, the process got better and better."

Read his whole article thoroughly: http://joelonsoftware.com/articles/fog0000000332.html
Anders Holtsberg Send private email
Thursday, December 25, 2008
 
 
Don't quit your job yet, read the "Getting to yes" book by Roger Fisher, William Ury and Bruce Patton


Tahar
Tahar Ouhrouche Send private email
Friday, December 26, 2008
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz