|
... while I normally give a once over to any checkins my coworkers make to code bases I'm working on, the only time I'm aware that my code was reviewed by someone else was shortly after starting my first ruby project where the lead dev complimented me for getting my style right instead of trying to jam C#/Java into .rb files. Even if I wasn't making a conscious effort because people writing C# in Java files or vice-versa is a pet hate of mine, at that point about 90% of what I'd done was to find a snippet of his code doing something equivalent, and then copy/pasting it with variables changed. OTOH the person he was working with before is near the top of my hate list for writing Java# I can understand him being a bit twitchy about bringing on another .NETer who'd never touched ruby before.
Did you ever see history portrayed as an old man with a wise brow and pulseless heart, waging all things in the balance of reason?
Is not rather the genius of history like an eternal, imploring maiden, full of fire, with a burning heart and flaming soul, humanly warm and humanly beautiful?
--Zachris Topelius
Training a telescope on one’s own belly button will only reveal lint. You like that? You go right on staring at it. I prefer looking at galaxies.
-- Sarah Hoyt
|
|
|
|
|
Code reviews are mandatory with medical devices. And yes they slow the process down, having participated in a few, ok a lot, of review processes they are a necessary evil. It would be devastating to a patient who’s condition was overlooked because of a “slap to the forehead, duh” line of code.
As was stated a few times here, once there is a familiarity of each other’s code it can go rather painlessly. And it does give one pause to consider that what they are writing because it will be looked at by other coders.
It does seem though when there is a new member to the process you sometimes get the apostrophe cops who nitpick the comments or attempts to engage the tabs vs. spaces, double slash or slash star type crap. There is usually one curmudgeon who will direct them to the company's coding standard and attempt to get the new member to calm down and focus on the code.
It was broke, so I fixed it.
|
|
|
|
|
We also inspect every single work byproduct using CodeCollaborator. I have mixed feelings about it. On the one hand if it is done well with the right people it's a great way to learn and prevent some but not all bugs. On the other hand not only do you have the stylistic police but you also have a few people who feel that they always must demonstrate their superior knowledge by making comments that add no value or are simply personal preference. Another thing that often happens is these same people like to expand the scope of the work product. Often reviews result is weeks of rework fixing issues that could either be deferred or ignored. Management has designated only certain people to be moderators so you can't avoid having some of these windbags involved. They often are the people who 10 years ago were practitioners and still think they know exactly how it all works. So you often spend a significant amount of time chasing ghosts for them because their recollection of how it used to be doesn't match the current reality. Other projects I have been on have used reviews very successfully but the effectiveness on my current project is questionable.
|
|
|
|
|
I worked for a medical device company, and we NEVER did code reviews. I've never heard of code reviews being an actual requirement, but more of a "strongly recommended" practice.
If I were in charge, I would've definitely included code reviews.
|
|
|
|
|
Actually there is no concept of code review,
but for Project Leader it is "we do code review regularly..."
|
|
|
|
|
We've introduced code reviews as part of the check-in process just a few years ago, and I must say I'm really happy that we did. Not only does it spread the knowledge around, we also regularly spot (potential) issues that the developer missed. And sometimes, by explaining things to someone else, you realize that it's all wrong in the first place! So it really benefits us, and I can definitely recommend this to others!
|
|
|
|
|
I came across this practice recently (since around one year and a half) but I'm really happy we decided to implement it.
More than the eventual bugs found it promotes the technical discussion between team members and after a while everyone perfectly understands each other code.
Another great thing that comes from this is that everyone will specifically know about everything. This is specially important when someone leaves the team temporarily of definitively.
Yet another advantage for those clumsy coders that just write code with the bare minimum style effort... as they know the code will later be read by someone else and probably will bounce back they will be more careful about more that making it work.
Bottom line it takes time... a lot of time, specially at the beginning, but as team members get used to each other code it get much easier.
If you want to try it, you really have to embrace it and give it a 6 month minimum trial period.
Cheers!
|
|
|
|
|
makes for a poor review ...
Could explain why agile development never took on here
|
|
|
|
|
Agile is a mindset rather than a process.
Much of the time I work alone too - but to a backlog, sprint by sprint, using TDD, continuous integration and metrics/static analysis to make sure I don't unwittingly do anything stupid. I also review every change in detail before committing it, and reject them if necessary (in that it probably helps tend to be a pretty strong critic of my own work, and a bit of a perfectionist).
Despite all that "overhead", I've found that I'm working faster now than I ever have. As a bonus, some of the practices I now use have led to our main codebase (around 400kLOC in size now) becoming less (rather than more) complex over time. As a result it's far more fun to work on than a 10 year old codebase has any right to be.
Hence my view that agile practices lead to far more efficient ways of working than the ways I used to do it.
Anna
Tech Blog | Visual Lint
"Why would anyone prefer to wield a weapon that takes both hands at once, when they could use a lighter (and obviously superior) weapon that allows you to wield multiple ones at a time, and thus supports multi-paradigm carnage?"
|
|
|
|
|
Agree, just joking about pair-programming
|
|
|
|
|
I have two chairs by my desk. When I feel like doing a bit of pair programming I jump across to the other one.
Oh and talk to myself constantly.
Anna
Tech Blog | Visual Lint
"Why would anyone prefer to wield a weapon that takes both hands at once, when they could use a lighter (and obviously superior) weapon that allows you to wield multiple ones at a time, and thus supports multi-paradigm carnage?"
|
|
|
|
|
I agree with you but...
"I also review every change in detail before committing it, and reject them if necessary"
This is cool and surely a good practice although the results are way better if someone else looks at our code. Doesn't matter how perfect we think we did it, there's usually something else or some other way we could have done. Even if it really doesn't matter at the end it's valuable knowledge that we get.
I've worked alone (and still do on parallel projs) but working with and towards teams is, IMHO, way way better
|
|
|
|
|
Of course. A second pair of eyes is always better - but when there aren't any others around at the time the responsibility by necessity falls on us alone.
As such I'd argue that to work alone you need to be more disciplined - not less.
Anna
Tech Blog | Visual Lint
"Why would anyone prefer to wield a weapon that takes both hands at once, when they could use a lighter (and obviously superior) weapon that allows you to wield multiple ones at a time, and thus supports multi-paradigm carnage?"
|
|
|
|
|
|
I think this is the most significant element of the Agile development methodology. So It's having huge benefit for us.B'cos it always gives huge impact for the maintain the clean and clear re-factored code base for our project. If you're not using this practice, please try to start it from today itself.
|
|
|
|
|
|
I answered the survey replacing "we" with "I". It comes down to the same though, I rarely see any issue with the brilliant work I created. If I should say so myself - I do marvelous work. Pad on the back for me, yay.
|
|
|
|
|
Upon reviewing your comment, I found a defect. "Pad" should be "Pat". Reviews can be helpful--even if you do it on your own work.
|
|
|
|
|
Because it's a bit difficult to argue with yourself as a one-man-band...
I do however, smack myself round the face when I realize I've done something stupid. Does that count?
Those who fail to learn history are doomed to repeat it. --- George Santayana (December 16, 1863 – September 26, 1952)
Those who fail to clear history are doomed to explain it. --- OriginalGriff (February 24, 1959 – ∞)
|
|
|
|
|
So I'm not the only one with the lonely head-slap habit ... upvoted for making me feel part of a community
|
|
|
|
|
Pretty much the same. I do use FxCop/CCM/CodeRush/etc. for checking code quality.
Getting information off the Internet is like taking a drink from a fire hydrant.
- Mitchell Kapor
|
|
|
|
|
OriginalGriff wrote: Because it's a bit difficult to argue with yourself as a one-man-band...
Not difficult at all actually. It's usually the best conversation I'll have on any given day. Who else wants to hear about arrays that reference arrays and such nonsense?
"Go forth into the source" - Neal Morse
|
|
|
|
|
Word.
The other week I was digging into some code I wrote four years ago and wondering why I had written it like I did.
At least it shows that I've improved rather than become worse.
This space intentionally left blank.
|
|
|
|
|
The hell with comments I want to know why I wrote the code, I can see what it does
Never underestimate the power of human stupidity
RAH
|
|
|
|
|
I've had that problem with code I wrote last week never mind last year.
|
|
|
|