|
Beyond a certain point you cross over from something being obviously crap, to "I like style X better than style Y"; at which point the main thing you're doing becomes adding noise to your source control diffs? Does it matter if other devs are still working on it, or if it's all you now?
Does having a tool like Resharper, whose utility in finding stuff the compiler warnings miss is badly degraded if it's spazzing out over style issues, affect where you draw the line? What if only part of the team has the tool?
3x12=36
2x12=24
1x12=12
0x12=18
|
|
|
|
|
Dan Neely wrote: Where do you draw the line?
You draw the line through all of their code and start over from scratch. 
It was broke, so I fixed it.
|
|
|
|
|
In a professional setting, you should use your company's coding standards to draw the line. If they don't have one, work with the team to develop one you like.
As a hobbyist, I tend to use the SharpDevelop Guidelines (PDF)[^]. About the only thing I change is I always put a brace on a newline instead of only on classes and interfaces. So, my typical if/then statement would be:
if (a + b)
{
}
instead of:
if (a + b) {
}
Sometimes I slip back to Hungarian notation for controls and variables, but I usually catch myself early enough to adjust the source accordingly. 
Flynn
|
|
|
|
|
|
I hear you. That's why I'm glad I only code for me. I can keep to my standards and not want to strangle the people around me. 
Flynn
|
|
|
|
|
A decent chunk of the stuff resharper is picking on in my current code base is stuff that made perfect sense when I was writing in .net 1.1. eg intelisence didn't kick in for members unless you typed this.; so my old code is blanketed in that verbose idiom.
What I'd really like would be a diff tool that would let me flag checkins to be ignored by default and be smart enough to display real code changes when crossing it. I suspect that doing so would be impossible in non-trivial cases however.
3x12=36
2x12=24
1x12=12
0x12=18
|
|
|
|
|
Dan Neely wrote: What I'd really like would be a diff tool that would let me flag checkins to be ignored by default and be smart enough to display real code changes when crossing it. I suspect that doing so would be impossible in non-trivial cases however.
I suspect you may be right.
Flynn
|
|
|
|
|
Maybe it could compile the code, and run through some variables and expected output for the changed methods. If the outputs change from known input/output set that was ran 'last time' the code was checked in, then the code has changed, otherwise the code just 'looks different'.
hmm, an actual reason to bother with unit testing......
----------------------------------------------------------------------
Get super cheap unlimited backup for 5 computers with this 'special' link: http://www.lushbackup.com?refid=matesrates[^]
|
|
|
|
|
Not neccesarily. If there is an optimization on the time or memory used, the expected input/output should not change, and the change is useful and important nevertheless.
|
|
|
|
|
I have a co-worker who formats all his code like this:
if (a && b) {
printf("Do something.");
}
(Notice the closing brace.)
It drives me absolutely batty! 
WE ARE DYSLEXIC OF BORG. Refutance is systile. Your a$$ will be laminated.
There are 10 kinds of people in the world: People who know binary and people who don't.
|
|
|
|
|
You're lucky.
The code I'm facing most often looks like this:
if ((((condition1 || condition2) && condition3) ||
condition4 || (condition5 && condition6 || condition7))
&& (condition8 || condition9)) || condition10)) {
}
Whether or not the closing brace is aligned correctly does not matter at all, becasue having to scroll over several 1000 lines of code utterly and completely eliminate all purpose from formatting rules.
|
|
|
|
|
I generally don't mess with style unless it is totally messed up. In which case I check-out, reformat the whole document, check in with the comment that all I did was reformat. Then on with code changes.
As a general rule though all code should be formatted in the default style within VS. The real formatting issues here are in the SQL stored procedures where we seem to have many distinctly different styles to the point where you can tell who wrote what by the way it's formatted. I've tried to get people to standardize on any of the formats, but have so far received no support on that. If only there was a tool that did as good of a job formatting SQL as VS does with C# code.
|
|
|
|
|
Yeah, were I to force a standard I'd probably go with resharpers (combined with getting all .net devs a copy of it); the only place I'm aware of it not matching with what VS does automatically in C# is that it wants to eat the _ in event handlers. It does add default styles for various other things (public/private/etc members); and points out chunks of unneeded code (this., superfluous casts, unneeded .ToString() calls, the verbose event handler attaching method needed in older versions of .net, etc). Stuff like that is where most of the potential changes would come from.
Enforcing a consistent style across code makes it easier to read, and while not entirely what I'd've picked all the defaults are reasonable, but the initial implementation pass can easily result in a checkin with enough changes that automatically diffing past it becomes nearly impossible. One example from a file I'm doing documentation on atm is ~300 suggested changes in ~2500 lines of code (closer to 2000 if refactored to use the autoproperties not available when it was originally written). That's only from the warning (yellow) list, there're a number of lesser code cleanup items (green) but I don't know how to get a total count. Even spread across multiple checkins (remove redundant code, renames, code simplification/cleanup) something like that becomes a real mess in history.
3x12=36
2x12=24
1x12=12
0x12=18
|
|
|
|
|
Jeremy Hutchinson wrote:
As a general rule though all code should be formatted in the default style within VS.
Amen! 
WE ARE DYSLEXIC OF BORG. Refutance is systile. Your a$$ will be laminated.
There are 10 kinds of people in the world: People who know binary and people who don't.
|
|
|
|
|
PL/SQL Developer has a "Beautify" option... of course, that client is only for Oracle's PL/SQL databases.
|
|
|
|
|
If I can read the code and understand it whatever style is fine. I don't care about casing or variable names. I've seen some people blow a gasket over naming conventions and such. To me it's just not that important.
What the code is doing matters. How it looks, not so much.
|
|
|
|
|
See my new signature. 
Martin Fowler wrote: Any fool can write code that a computer can understand. Good programmers write code that humans can understand.
|
|
|
|
|
I'll second that; I've been maintaining a puddle of pooh (in VB.net no less) for the last year. I try to only fix bugs, not style.
|
|
|
|
|
When the crap level goes past my knees, I switch from mucking out the stall myself to kicking the ass of the person responsible.
If I'm already annoyed that day, it may happen sooner.
Software Zen: delete this;
|
|
|
|
|
Right at the start... Log a project level bug, mention the files that are affected. And the general reason... e.g. not according to coding standard, or whatever.
"Program testing can be used to show the presence of bugs, but never to show their absence."
<< please vote!! >>
|
|
|
|
|
Rewriting someone elses code without being specifically asked to do so is bad
modified 20-Oct-19 21:02pm.
|
|
|
|
|
If the fix is simple, I generally leave it alone, but if it is not trivial, then I reformat it to make it readable. The previous programmer may have created a house of cards, if you break it, all eyes are on you. So I want the code in a form I can verify for myself that it has a chance of working and be easy to fix if it turns out there was for than madness to the previous programmer's method.
Psychosis at 10
Film at 11
|
|
|
|
|
Stylistic opinions don't warrant code changes unless the entire team agrees. A team of one, or someone with the authority to dictate what they team wants, will find it easy to get agreement.
If the team wants some tool to work well, and it has stylistic requirements, then they are agreeing to adhere to its stylistic requirements.
patbob
|
|
|
|
|
What brought you to look at the code?
If obfuscation hides functional problems, start refactoring. If I've more time, I work deeper than strictly necessary - often I see problems not apparent when I started.
A bigger issue you allude to: "What if only part of the team has the tool?".
If you're in the same code base the team needs access to the same tools. Period. Hopefully they all have access to the same training and materials, if need be. Nothing worse than being second class citizen in a project, or worse, having to supplicate the gods of the team, just to get the days tasks done.
|
|
|
|
|
James W Taylor wrote: If you're in the same code base the team needs access to the same tools. Period
Agree completely. The problem is my employer has reached the size where they have enough bureaucracy we can't just go to purchasing and say "X Costs $250 and will save me at least 10 hours of work over the course of the project, more than paying for itself", but not enough to have gotten stuff beyond the basics (eg visual studio itself) on the pre-approved software list.  It took my coworker several months of working the bureaucracy to get me a copy of Resharper. 
3x12=36
2x12=24
1x12=12
0x12=18
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
