Is the Code Review Parallel Programming or Singleton?
-
A pig walked into a bar with a tank of liquid nitrogen and forty-two CListCtrls... Hmm... Nah, doesn't work.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)And said "It's PORNO time" ... Yes it does. Kinda. In my head :)
Sort of a cross between Lawrence of Arabia and Dilbert.[^]
-Or-
A Dead ringer for Kate Winslett[^] -
Senior people review less senior people's code. When equal level peers are dependent on each other's code, they may review each other. Saying something like "let everyone review everyone else's code" is a waste of time if, for example, an intern is reviewing an architects library, etc, unless it's for learning experience.
"I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
wizardzz wrote:
an intern is reviewing an architects library, etc, unless it's for learning experience.
This really depends on the place. My internships dealt with code that had never had a pure programmer work on it, everything was done by engineers who learned to code. While everything worked, some of it was a rather marvelous mess and a great deal of it was massively improved by the time another intern and I were done with it. It's probably rare, at least I'd like to think it is, but it would have been a lot easier to deal with had they actually allowed us to look at all of the code to see what could be overhauled when we started, rather than spending half our time working through half hacked together pages until we found consistent patterns we could correct. The only real advice I have for code reviews is to have people actually look at the code before the meeting. Doesn't have to be much, just enough to have a clue what's supposed to go on. The one official one I went through was remarkably painful compared to the more impromptu meetings that the other intern and I had setup because half the room had no clue what was even supposed to be going on. Though it also helps if they actually care. That was a bit of a problem with the official meeting as well, the code had remarkably well defined territories within it. But that's a culture problem.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
I've got plenty of time to do it just send the code and a check.
"Life can only be understood backwards, but it must be lived forward." Kierkegaard, Søren
-
42
CQ de W5ALT
Walt Fair, Jr., P. E. Comport Computing Specializing in Technical Engineering Software
52 is the new 42. :)
"Life can only be understood backwards, but it must be lived forward." Kierkegaard, Søren
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
Quis custodiet ipsos custodes? That is indeed the question.
¡El diablo está en mis pantalones! ¡Mire, mire! SELECT * FROM User WHERE Clue > 0 0 rows returned Save an Orange - Use the VCF! Personal 3D projects Just Say No to Web 2 Point Blow
-
wizardzz wrote:
an intern is reviewing an architects library, etc, unless it's for learning experience.
This really depends on the place. My internships dealt with code that had never had a pure programmer work on it, everything was done by engineers who learned to code. While everything worked, some of it was a rather marvelous mess and a great deal of it was massively improved by the time another intern and I were done with it. It's probably rare, at least I'd like to think it is, but it would have been a lot easier to deal with had they actually allowed us to look at all of the code to see what could be overhauled when we started, rather than spending half our time working through half hacked together pages until we found consistent patterns we could correct. The only real advice I have for code reviews is to have people actually look at the code before the meeting. Doesn't have to be much, just enough to have a clue what's supposed to go on. The one official one I went through was remarkably painful compared to the more impromptu meetings that the other intern and I had setup because half the room had no clue what was even supposed to be going on. Though it also helps if they actually care. That was a bit of a problem with the official meeting as well, the code had remarkably well defined territories within it. But that's a culture problem.
Distind wrote:
The only real advice I have for code reviews is to have people actually look at the code before the meeting. Doesn't have to be much, just enough to have a clue what's supposed to go on. The one official one I went through was remarkably painful compared to the more impromptu meetings that the other intern and I had setup because half the room had no clue what was even supposed to be going on.
Though it also helps if they actually care. That was a bit of a problem with the official meeting as well, the code had remarkably well defined territories within it. But that's a culture problem.I agree here heavily. I don't think code reviews necessarily need a formal meeting attached. At least that's been my positive experience, an informal gathering of 2 or 3 people at one's desk, after all have read and understood the code / project / requirements, seems to be effective from what I remember. The part regarding, if they actually care is very important, too. That's why I don't recommend too senior of people reviewing too low level code, as well as peers that are distantly positioned on a project. People are busy, I can understand why sometimes the code isn't looked at too deeply.
"I have a theory that the truth is never told during the nine-to-five hours. " — Hunter S. Thompson
-
A pig walked into a bar with a tank of liquid nitrogen and forty-two CListCtrls... Hmm... Nah, doesn't work.
Proud to have finally moved to the A-Ark. Which one are you in?
Author of the Guardians Saga (Sci-Fi/Fantasy novels)Very creative, you should be a writer!
Help a brotha out and vote Managing Your JavaScript Library in ASP.NET as the best ASP.NET article of May 2011.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
I think it should be responsibility of one person/team dedicated for the task - for better results - if you need to ship your code ASAP you are not going to sort out the mess in other's even if boss says so, on being pressed you will complete the formality and move on. Others can point out the issue if they found any anonymously - until unless direct intervention is required-, this will make team go smooth - my code is my pride-. Edit--- Looks like there is some issue with signature rendering so removed it from the post.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
Usually, you review each other's code. If your application processes highly sensitive data, it might be a good idea to hire consultants that specialize in code security.
-- Kein Mitleid Für Die Mehrheit
-
I think it should be responsibility of one person/team dedicated for the task - for better results - if you need to ship your code ASAP you are not going to sort out the mess in other's even if boss says so, on being pressed you will complete the formality and move on. Others can point out the issue if they found any anonymously - until unless direct intervention is required-, this will make team go smooth - my code is my pride-. Edit--- Looks like there is some issue with signature rendering so removed it from the post.
Amar Chaudhary wrote:
you are not going to sort out the mess in other's even if boss says so, on being pressed you will complete the formality and move on.
Are you saying that if you were aware of a flaw in a colleagues code, you would not mention it?
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
IMHO everyone should review everyone's code; it is more in the spirit of the team - when reviewing someone's code it isn't a 'bad' thing to find issues, it's a positive thing. less experienced programmers get to see other's code more frequently than otherwise, and having to review it makes them take the time to understand and learn more experienced programmers are sometimes a little, well, set in their ways. Having others review their code is sometimes a learning experience for them, too any developer may look at someone's code and ask themselves "I wonder why they did it this way rather than that way?" If the question is asked, at least one of the participating parties learns something standards are a fluid thing - so multiple code reviewers allow discussions (aka raging flame-wars) to take place regarding standards and, hopefully, consensus can be reached Having one person do the reviews is prone to problems - the person leaves/is off sick/ is a loony/ with one person doing reviews, who reviews their code?
MVVM# - See how I did MVVM my way ___________________________________________ Man, you're a god. - walterhevedeich 26/05/2011 .\\axxx (That's an 'M')
-
Amar Chaudhary wrote:
you are not going to sort out the mess in other's even if boss says so, on being pressed you will complete the formality and move on.
Are you saying that if you were aware of a flaw in a colleagues code, you would not mention it?
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
No it means : when you have lack of time then you are not going to invest it in finding flaws of others code.
-
I have a new team forming up and we want to decide on the best way to formalize code reviews. Is the Code Review the responsibility of one person or the whole team? Is each, or certain, team member(s) allowed to review others' code or is there only one person who does the reviews? If it's only one person, then who reviews his/her code? I've heard of both in practice and some of the teams where I'm at use both patterns. I'm just looking for insight as to what everyone thinks is the best way to go and why.
- Arcond
Are you sure you mean parallel programming? Sounds more like pair programming, to me. Does someone not think that "pair" is a snazzy enough word?
I wanna be a eunuchs developer! Pass me a bread knife!
-
No it means : when you have lack of time then you are not going to invest it in finding flaws of others code.
If you worked for me and failed to participate fully in a procedure that had been set out, you wouldn't work for me.
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
-
If you worked for me and failed to participate fully in a procedure that had been set out, you wouldn't work for me.
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
As a boss please let me know that today is Friday I have a shipping target for Monday morning and I am busy finishing it, what would you like more : 1. I finish my target and deliver on time. 2. I find 10 issues in somebody's code and missed the target. My point is that if a responsibility is shared among multiple people with equal authority then nobody is actually responsible. My Startup!!!!
Profile@Elance - feedback available too -
As a boss please let me know that today is Friday I have a shipping target for Monday morning and I am busy finishing it, what would you like more : 1. I finish my target and deliver on time. 2. I find 10 issues in somebody's code and missed the target. My point is that if a responsibility is shared among multiple people with equal authority then nobody is actually responsible. My Startup!!!!
Profile@Elance - feedback available tooAmar Chaudhary wrote:
My point is that if a responsibility is shared among multiple people with equal authority then nobody is actually responsible.
That is a totally different point than that which you first made. My point still stands. Assuming I am the boss. If I tell you you will do code review, you will do code review or you won't work for me. If you feel that it will prevent you from hitting a deadline then you discuss that with your immediate line manager. Whatever their decision (make sure to get it in writing :) ), you adhere to it, or you don't work for me. If that decision causes the deadline to be missed then that's what it is.
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
-
Amar Chaudhary wrote:
My point is that if a responsibility is shared among multiple people with equal authority then nobody is actually responsible.
That is a totally different point than that which you first made. My point still stands. Assuming I am the boss. If I tell you you will do code review, you will do code review or you won't work for me. If you feel that it will prevent you from hitting a deadline then you discuss that with your immediate line manager. Whatever their decision (make sure to get it in writing :) ), you adhere to it, or you don't work for me. If that decision causes the deadline to be missed then that's what it is.
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
Henry Minute wrote:
If I tell you you will do code review
This is assigning responsibility to one single person, it is effective but declaring everybody will do the code review of peer programmer will lead to abuse of process. And that's what I said in initial post.
-
Henry Minute wrote:
If I tell you you will do code review
This is assigning responsibility to one single person, it is effective but declaring everybody will do the code review of peer programmer will lead to abuse of process. And that's what I said in initial post.
Amar Chaudhary wrote:
This is assigning responsibility to one single person,
No it is not. It is you deliberately assigning meaning to a phrase that was not originally intended. If I, as a manager decide that code reviews will be done, they will be done. Doesn't matter if by one or by all. If someone pays your wages and they assign a valid work related task to you, then you do it or leave. Either by your own volition, or otherwise. It matters not one jot if the task is the best method in your opinion. You said originally that you would decide to do code review or not. You were wrong in that statement, for reasons I have already stated.
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.” I wouldn't let CG touch my Abacus! When you're wrestling a gorilla, you don't stop when you're tired, you stop when the gorilla is.
-
Are you sure you mean parallel programming? Sounds more like pair programming, to me. Does someone not think that "pair" is a snazzy enough word?
I wanna be a eunuchs developer! Pass me a bread knife!
The parallel bit was my sad attempt at making a metaphor for design patterns as real world code review exercises. Parallel because you're utilizing all your cores (devs) vs singleton which is intentionally resource restricting (i.e. one person). If we were practicing pair programming we could eliminate the code review all together as that's the purpose of pair programming; doing the code review up front while you're coding. :)
- Arcond
-
52 is the new 42. :)
"Life can only be understood backwards, but it must be lived forward." Kierkegaard, Søren
913 x 613 = 4213 Which just show why everything seems to go wrong. The universe is based on an unlucky number.
