|
I've been a developer for several decades. I get value from my code being reviewed by others.
My ideas on reviewing of other people's code should be obvious.
====
On a related note, one of my observations is that every contributor will search until they find something to criticize. One of my review techniques (for my code submissions) is to make sure that there is at least one blatant mistake that everybody can find, which can be "fixed" with a trivial action on my part; else if your submission is truly close to perfect, you will get several new requirements to add wheel spinning features that have little (or negative) value.
Windows 8 is the resurrected version of Microsoft Bob. The only thing missing is the Fisher-Price logo.
- Harvey
|
|
|
|
|
A few of my thoughts:
1) Code reviews are good.
2) If you think your code doesn't need to be reviewed - Then it needs to be reviewed
3) As you said, code reviews are for everyone, Say you write perfect code, what better way for someone else to learn something they didnt know of
4) More and more, static analysis tools can check for coding standards, unfortanately most of the comments in reviews relate to naming conventions and comments :/
5) I like to focus on correctness of code and design for integration with the system and future maintainability.
To know and not do, is not yet to know
http://www.codeofthedamned.com
|
|
|
|
|
What a silly idea. I'm a man, I know what I am doing and there is no reason for anyone else to look at my perfect code. I've never made a mistake and I never will.
|
|
|
|
|
You're married... when was the last time you were right about anything???
|
|
|
|
|
That is not related to marriage. I know this because I'm in fact not married but living in sin and still incorrect about everything.
|
|
|
|
|
|
Given the amount I've heard lately about her preferences in diamond settings I suspect she would prefer tomato
|
|
|
|
|
You're getting the hint then? Wait, are you currently in custody up here at Indooroopilly??? (If this makes no sense, google Indooroopilly diamond theft.)
|
|
|
|
|
_Damian S_ wrote: You're getting the hint then? Wait, are you currently in custody up here at Indooroopilly??? (If this makes no sense, google Indooroopilly diamond theft.)
Josh would need several layers of sun tan to be the diamond ring pooper man.
Michael Martin
Australia
"I controlled my laughter and simple said "No,I am very busy,so I can't write any code for you". The moment they heard this all the smiling face turned into a sad looking face and one of them farted. So I had to leave the place as soon as possible."
- Mr.Prakash One Fine Saturday. 24/04/2004
|
|
|
|
|
True that... maybe it was his brilliant disguise?
|
|
|
|
|
_Damian S_ wrote: (If this makes no sense, google Indooroopilly diamond theft.)
Sorry, I was in the bathroom with constable smellmyturd
|
|
|
|
|
Get back into the kitchen with the other girlies
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
But seriously, either your workmates think like this or they are scared of being shown up by the new guys
|
|
|
|
|
A little of both, I think.
Many closed minds.
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
I've had excellent experience with Code Reviews.
We have found a number of bugs that would have gone unnoticed if not for code reviews, and I'm a strong advocate of having code reviews.
"Real men drive manual transmission" - Rajesh.
|
|
|
|
|
It's a great way of learning more about what it is that you're making, which can help you to do your own bits easier.
I wanna be a eunuchs developer! Pass me a bread knife!
|
|
|
|
|
Suggest you agree the standards and process in advance - by discussion not by dictat.
|
|
|
|
|
Code reviews can be a very good idea, provided that the standards the code is to adhere to are agreed in advance, and everybody remembers that the purpose is to improve code quality, not score points by nit-picking every tiny detail to make the author look like an idiot and everyone else feel "big and strong".
Used well, they are an definite plus: used badly they are a tool for workplace bullying and can destroy morale in a department.
|
|
|
|
|
_Maxxx_ wrote: and down if you think they are the devil's work!
not fair! Downvoting is disabled.
I guess it kinda depends on how, when, etc the code reviews are done.
|
|
|
|
|
_Maxxx_ wrote: My belief is that code reviews will help ensure standards are kept, I love reviews, but not for the sake of a "standard". I could not care less about whether you think each instance member needs be prefixed with this. , whether private members are preceded with the term "private".
If it does not add value (and only cost), then the best practice would state to not enforce the standards.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
well there are standards, and there are standards.
My view of any standard is that it should have a reason for existing.
If there is a good reason for using this. over not using this. then it should be adopted as a standard. If there is no compelling reason then there shouldn't be a standard.
The important thing to remember, though, is that one man's "but its obvious" is another man's "Wow! I didn't realise" - so having standards with explanations is a good idea - especially for those less experienced in that area of code.
A good example, I think, is in Wpf - where many a standard says "thou shalt not write any code behind"
well that is obviously bollocks (IMHO) and should be more like "thou shalt not put business logic in the code behind".
And that is where a code review comes in handy, as then more than one pair of eyes can look at any code and decide if it is business logic or not.
MVVM # - I did it My Way
___________________________________________
Man, you're a god. - walterhevedeich 26/05/2011
.\\axxx
(That's an 'M')
|
|
|
|
|
_Maxxx_ wrote: well there are standards, and there are standards. A standard means that you follow Microsofts' guidelines[^], as opposed to inventing your own "standard". Nine of ten times, the terms coding convention and code reviews means a subset of forementioned document, without any argumentation and a lot of fruitless discussions.
_Maxxx_ wrote: My view of any standard is that it should have a reason for existing. FxCop will give it to you as soon as you break one of those rules. That should give enough indication on whether or not the deviation is logical or not.
_Maxxx_ wrote: If there is a good reason for using this. over not using this. then it should be adopted as a standard. Stop talking about it, follow the standard, omit it. The same goes for declaring things "private"; learn what the default access modifier is for your language, and start following the standard. Omitting the obvious makes code a lot more readable.
..and that's why we wanted that standard in the first place.
_Maxxx_ wrote: having standards with explanations is a good idea - especially for those less experienced in that area of code. Making code readable does far more to help the less experienced. And any standard without argumentation should be dismissed without argumentation.
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
|
|
|
|
|
Do I think code reviews should be done and can be useful? Yes...
However, before a code review can be done to
_Maxxx_ wrote: ensure standards are kept , then said standards must have been developed ahead of time and the reasoning for the standard should be described in the standard. "Because I said so" is not a reason.
If you are going to write code that is going to be reviewed, be prepared to both document and explain in person any processing that others may not have seen.
In the late '80s I was involved in rewriting two systems to move them to a new platform. Code review was in place for both rewrites.
One of the 'standards' was: all file names must be 9 characters and the routine name in the file must match the file name. Since the first 3 characters described the system, that left 6 characters to define a meaningful file and routine name. This standard was quickly abandoned for routine names because they were much more cryptic than meaningful; this was for a system with over 500 routines in a single executable.
And the reasoning for the standard? The old platform was limited in the length of a file name size, and the standards developers didn't check the system capabilities on the new platform.
While reviewing code (FORTran), I encountered something I had never seen before (a multiple return) and asked the developer about it. His reply, "It's a multiple return". He either could not or would not describe it in any other terms. I finally replied, "And telling me a car is a car is only useful if I understand what a car is." At that point, he said, "I'll rewrite the code". Again, nothing wrong with the code, but the developer could not explain what is was doing.
Just my thoughts...
Tim
|
|
|
|
|
_Maxxx_ wrote: The other developers think it is a waste of time except for new developers
having their code reviewed (presumably until they can be trusted)
They are wrong.
_Maxxx_ wrote: My belief is that code reviews will help ensure standards are kept, enhance the
learning of all the developers, and help drive any changes in standards.
Standards mean little in terms of actual quality unless reviews, not just code reviews, are taken seriously. Reviews of the entire process lead to many types of improvements where as style guidelines have a very small impact.
They do increase learning. One is exposed different expressions immediately rather than just via maintenance.
It also insures adherence to requirements in that reviews at least have the possibility that they might cause a failure of a requirement in another part of the system, before that system reaches testing (which would hopefully demonstrate it regardless.)
|
|
|
|
|
http://www.youtube.com/watch?v=Ja2__Q52GAU[^] (Video, SFW)
If your actions inspire others to dream more, learn more, do more and become more, you are a leader.-John Q. Adams You must accept one of two basic premises: Either we are alone in the universe, or we are not alone in the universe. And either way, the implications are staggering.-Wernher von Braun Only two things are infinite, the universe and human stupidity, and I'm not sure about the former.-Albert Einstein
|
|
|
|