• 0

How do you feel about others refactoring your code?


Question

Until now, all the team projects I've worked on had no coding conventions whatsoever, whether at school or at my internship. Now, when I was at my internship, I tried to stick with the de facto conventions present in the code, but there are times where I'd do my way instead.

For instance, I think in a C-based language, if-else statements should be written like so:

if (condition) {
	statements
}
else {
	statements
}

instead of the more traditional

if (condition) {
	statements
} else {
	statements
}

And that one-line if-statements are unacceptable. I've too often seen this:

if (condition)
	statement 1
	statement 2
statement 3

Where indentation suggests statement 2 is conditional, when it's not. This is incredibly misleading, and yes, I've seen it cause bugs. So I always refactor that code as:

if (condition) {
	statement 1
}
statement 2
statement 3

I also think predicates should be named isSomething(), and other such conventions.

Now, I tend to take it pretty personal when someone in the team changes function names, layout etc in my code, without discussing it with me first. However, even if we discuss it, there's no easy way to reach common ground on these matters. For instance, my school teachers think all methods should be preceded by such monstrosities:

/*************************************************
*   function name : add						  *
*   arguments :								  *
*	   int number1 : the first number to add	*
*	   int number2 : the second number to add   *
*   returns : the sum of number1 and number2	 *
*   author : Dr_Asik							 *
*   last modified : 6/01/09					  *
*   author's blood group : AB+				   *
*   more crap									*
*   even more crap							   *
*												*
*************************************************/
int add(int number1, int number2) {
	return number1 + number2;
}

...which are such a pure GIMMICK and an utter WASTE OF TIME, it makes me wanna hit someone.

How do you cope with projects where there are no established conventions? Above all else I think maintaining good, open relations with my team members is the most important, so I'm a bit embarassed when it comes to coding conventions. Either we adopt mine or someone else's, and it becomes a matter of who's able to prove his authority over the team better. I don't like it, especially if we're equal team members on the project.

Edited by Dr_Asik
Link to comment
Share on other sites

Recommended Posts

  • 0

I've only worked on a few projects that didn't have set coding conventions, and those cases I sat down for a few minutes with the rest of the team, found out what coding style the most people on the team like... and we stick to that.

I would refuse to work on a project that's a complete mis-match of coding conventions without any specific guidelines. If they don't have guidelines, but most people code one way and you decide to do it your own way definitely expect your code to get refractored, don't however refactor something just because you don't like it (e.g. the single line statement).

So basically, talk to them, also - give your teacher a slap :p

Link to comment
Share on other sites

  • 0

But it's hard to make compelling arguments about those things. People prefer a style because that's what they're used to. I can make a compelling argument that the traditional brace style

scope 
{
}

doesn't make any sense and one should always use

scope {
}

, but even if put it very convincingly, some people simply find the latter ugly and there's just no reasoning to be done.

Link to comment
Share on other sites

  • 0

Are you really complaining that you have to document functions and modules, or are you just complaining about the coding conventions your teacher has set? If it's the latter, you might as well get used to follow orders now.

Link to comment
Share on other sites

  • 0
But it's hard to make compelling arguments about those things. People prefer a style because that's what they're used to. I can make a compelling argument that the traditional brace style

scope 
 {
 }

doesn't make any sense and one should always use

scope {
 }

, but even if put it very convincingly, some people simply find the latter ugly and there's just no reasoning to be done.

I find the trailing brackets (code example 2) make it harder for me to find the beginning and end of the block. I try to stick with the conventions of the language otherwise.

I use C# and my code usually follows:

class Foo
{
}

void Function()
{
}

int Property
{
	 get {}
	 set {}
}

Link to comment
Share on other sites

  • 0

Everyone has their own personal preferences. For example, I generally put the following as a header in each file:

/*
* Project Name
* FileName.Ext - Desctiption of what file does
* $Rev$
*/

With $Rev$ being the SVN revision keyword. As for if statements, I use one line if statements if there's only one function call, example:

if (something) {
   doSomething();
   and(this);
   while(somethingElse()) {
	  doThis();
	  var++;
   }
}

if (something) doThis();

Link to comment
Share on other sites

  • 0

Honestly while while your example was way over the top, your teacher makes sense, As you will learn once you start taking over old projects with no documentation whatsoever, and you scratch your head wondering what the hell this is for and what that is for... and you spend hours and days working out how it all fits together instead of coding.

yeah sure you don't need to comment every tiny little bit, ut it's important for your own and others sake to at least document the important overall bits and main components.

Link to comment
Share on other sites

  • 0
Are you really complaining that you have to document functions and modules, or are you just complaining about the coding conventions your teacher has set? If it's the latter, you might as well get used to follow orders now.
Functions should be documented, but you shouldn't have to draw ASCII art all around it, and if comments do nothing else than state what is obvious in the code, they're useless clutter. Also mentioning last modified date is useless if you're using SVN where all the versioning data is automatically registered for you, in a much more reliable way (what if someone modifies the code and forget to update the "last modified" comment?).

Anyway, that was just an example. I don't really want to discuss coding conventions per say, but how does one deal with not having any official conventions on a team. Do you run votes? Argue over each convention? Elect an already existing coding convention document?

Link to comment
Share on other sites

  • 0

Normally the chief architect for the team decides what the project's coding conventions are. Usually he (or she) is the one that must coordinate all the programming subprojects so it is vital that the code is readable to them as well as the programmer. ASCII art is silly, agreed. I've found the best documentation is to properly name fields, functions, and classes based on what they're for.

bool b = false;
...
if(b)
{
	 ...
}

isn't going to be obvious later what you're checking even if you wrote it if it's a big project.

bool isPresent = false;
...
if(isPresent)
{
	 ...
}

is quite obviously checking to see if something exists. :)

(I'm a big fan of VS's documentation tagging, too. Typing "///" and entering what a function does is a breeze and you can build SDK documentation automatically later.)

Your teacher-style documentation,

/*************************************************
*   function name : add						  *
*   arguments :								  *
*	   int number1 : the first number to add	*
*	   int number2 : the second number to add   *
*   returns : the sum of number1 and number2	 *
*   author : Dr_Asik							 *
*   last modified : 6/01/09					  *
*   author's blood group : AB+				   *
*   more crap									*
*   even more crap							   *
*												*
*************************************************/

looks like the stuff my HS my programming teachers wrote. I let my IDE track that stuff, but I suspect many teachers learned before that information was available as metadata.

Link to comment
Share on other sites

  • 0

My thought : Not a single line of comment in my source files. I always begin designing a very solid model with a solid documentation detailing the different classes, methods, properties, functions etc ... before starting the implementation. In other words, I make comments before writing code. If I have to clarify something I go back to my model and clarify it there.

Advantages :

- Sources remain clean and compact.

- When a new guy joins the team, he doesn't have to dive in code and comments to know how everything works. He starts by studying the model and understanding the code without even seeing it.

- Even if this method seems painful it really makes team-work very easy.

Disadvantages :

- If you lose the documentation you can start crying.

- As for conventions I always hand the team members an "Always Remember !" paper with all the good practices and conventions we're gonna need (going from basic stuff to more complex and project-specific guidelines). These kind of problems should be definitely fixed the first day and if you find out that someone refactored your code then you have to advise the team leader because that means something's wrong with the project organization.

Hope it helps you :)

Link to comment
Share on other sites

  • 0

^ :p I'm not working on any projects at the present moment (preparing some boring IT stuff for summer, meh :/). But the second I find some motivation to work on some projects I have in mind I'll make sure I contact you to see if you're interested :)

Link to comment
Share on other sites

  • 0

i have just finished a project with only one other full time developer, and we had no set standards, but towards the end of the job during deployment we had some pretty heated 'discussions' about how code should look, and the way you should do things..

in the end we came up with a few rules:

1. using your 'style' immediately shows who has done that work, so it matters not

2. ALWAYS put code comments at the top of classes, methods, stored procedures, views etc

3. ALWAYS document changes with dates and reasons

4. don't change existing code for the hell of it

in short, we decided the style doesn't matter so long as it was commented and did the job..

but i see your frustration, i used to be like that too!

My thought : Not a single line of comment in my source files. I always begin designing a very solid model with a solid documentation detailing the different classes, methods, properties, functions etc ... before starting the implementation. In other words, I make comments before writing code. If I have to clarify something I go back to my model and clarify it there.

Advantages :

- Sources remain clean and compact.

- When a new guy joins the team, he doesn't have to dive in code and comments to know how everything works. He starts by studying the model and understanding the code without even seeing it.

- Even if this method seems painful it really makes team-work very easy.

Disadvantages :

- If you lose the documentation you can start crying.

- As for conventions I always hand the team members an "Always Remember !" paper with all the good practices and conventions we're gonna need (going from basic stuff to more complex and project-specific guidelines). These kind of problems should be definitely fixed the first day and if you find out that someone refactored your code then you have to advise the team leader because that means something's wrong with the project organization.

Hope it helps you :)

This is well and good in principle, but I'm sure you will agree it is time consuming and therefore how do you get your clients to pay for it?

Documentation is pretty much always the first thing a customer wants to chop out of a proposal (followed by testing!!!). This in itself is no big deal as it generally leads to a support contract but id like to hear how you do it?

Ta!

Link to comment
Share on other sites

  • 0
This is well and good in principle, but I'm sure you will agree it is time consuming and therefore how do you get your clients to pay for it?

Documentation is pretty much always the first thing a customer wants to chop out of a proposal (followed by testing!!!). This in itself is no big deal as it generally leads to a support contract but id like to hear how you do it?

Ta!

I'm sure E.Fahd will want to answer himself but my 2 cents is that the client shouldn't be telling you how to work. The documentation is your tool as a programmer and you use the tools you want to get the job done as efficiently and cleanly as possible. Of course, I'm talking about developer documentation (classes, modules, etc.). If the customer doesn't want any guide on how to use the software, that's his choice of course.
^ :p I'm not working on any projects at the present moment (preparing some boring IT stuff for summer, meh :/). But the second I find some motivation to work on some projects I have in mind I'll make sure I contact you to see if you're interested :)
Sure, give me a PM anytime !
Link to comment
Share on other sites

  • 0

Just wait until you work on a project as part of your job, where your family depends on your paycheck which is reflective of your performance, but you're fighting the endless battle of people that don't comment their code or follow the company coding convention.

For example, the team I work on is split geographically, we literally have people spread out all over the world. The team has a deadline approaching and you were tasked with fixing a bug 2 days before the deadline (this makes it your ass on the line), but there's no documentation, comments, and the code is a mess (doesn't follow the coding standards).

What are you going to do?

You could spend countless hours figuring it out on your own, but you don't want to do this as it's highly inefficient and your boss is likely to tell you to stop wasting time and just call the person. You pick up the phone and start dialing, but realize they live in a different part of the world, 12 hours ahead of you and it's 03:00 their time... you're stuck.

Now what? You know... you'll call the architect. He has to know, he helped design the damn thing. Unfortunately, the architect has chosen to take his vacation right around that time... you're stuck again.

What do you do? you spend 16 hours or more, if necessary and get the bug fixed. You're not happy about it... you're ****ed actually because you lost out on your time with your family because someone couldn't take the extra two minutes to comment their code or follow the company coding standard.

90% of development should be about the maintainability of the code... end of story (true story)

Link to comment
Share on other sites

  • 0

@BGM,

Actually when I say "Documentation" I'm referring to all the preliminary work you do before starting the proper implementation (including model design, problem analysis and algorithms analysis if there are any algorithms involved). The client has nothing to do with it since the benefits of this "step" remain internal and will never be seen in the final product.

As for the efficiency I like to think of it as QuickSort vs MergeSort. If you have a small project that won't evolve very much then just go with the quick way, but if you're leading a relatively big team, with different programming cultures (not everyone code the same way), different skill levels, etc ... then, overall, the strict way will turn out to be actually quicker.

Link to comment
Share on other sites

  • 0
Just wait until you work on a project as part of your job, where your family depends on your paycheck which is reflective of your performance, but you're fighting the endless battle of people that don't comment their code or follow the company coding convention.

For example, the team I work on is split geographically, we literally have people spread out all over the world. The team has a deadline approaching and you were tasked with fixing a bug 2 days before the deadline (this makes it your ass on the line), but there's no documentation, comments, and the code is a mess (doesn't follow the coding standards).

What are you going to do?

You could spend countless hours figuring it out on your own, but you don't want to do this as it's highly inefficient and your boss is likely to tell you to stop wasting time and just call the person. You pick up the phone and start dialing, but realize they live in a different part of the world, 12 hours ahead of you and it's 03:00 their time... you're stuck.

Now what? You know... you'll call the architect. He has to know, he helped design the damn thing. Unfortunately, the architect has chosen to take his vacation right around that time... you're stuck again.

What do you do? you spend 16 hours or more, if necessary and get the bug fixed. You're not happy about it... you're ****ed actually because you lost out on your time with your family because someone couldn't take the extra two minutes to comment their code or follow the company coding standard.

90% of development should be about the maintainability of the code... end of story (true story)

If there's an established coding convention and one can't have the decency of sticking to it, he should f***ing get fired. I haven't been in your situation (actually I've yet to finish my degree), but I can already feel your frustration. It's not just about aesthetics, it's about subtle bugs creeping in because some semantics are not understood well by certain team members or some changes have been made without any notice...
Link to comment
Share on other sites

  • 0

I normally stick to using coding standards of whatever workplace I am in. If I am a consultant and I am visiting a firm where I need to just jump in and do my thing then I follow their coding styles and if I am working in house I use the standards set at the base firm.

Link to comment
Share on other sites

  • 0
Just wait until you work on a project as part of your job, where your family depends on your paycheck which is reflective of your performance, but you're fighting the endless battle of people that don't comment their code or follow the company coding convention.

For example, the team I work on is split geographically, we literally have people spread out all over the world. The team has a deadline approaching and you were tasked with fixing a bug 2 days before the deadline (this makes it your ass on the line), but there's no documentation, comments, and the code is a mess (doesn't follow the coding standards).

What are you going to do?

You could spend countless hours figuring it out on your own, but you don't want to do this as it's highly inefficient and your boss is likely to tell you to stop wasting time and just call the person. You pick up the phone and start dialing, but realize they live in a different part of the world, 12 hours ahead of you and it's 03:00 their time... you're stuck.

Now what? You know... you'll call the architect. He has to know, he helped design the damn thing. Unfortunately, the architect has chosen to take his vacation right around that time... you're stuck again.

What do you do? you spend 16 hours or more, if necessary and get the bug fixed. You're not happy about it... you're ****ed actually because you lost out on your time with your family because someone couldn't take the extra two minutes to comment their code or follow the company coding standard.

90% of development should be about the maintainability of the code... end of story (true story)

Code comments should be the exception, not the norm. I don't want my source files decorated with 150+ lines of comments that could be inferred by variable and method names that actually made sense.

Most of the time, developers use comments as a crutch to explain code that should be self-documenting. Take a look at method-specific documentation. You'll have parameter documentation that just repeats exactly what the variable names are saying.

Does that mean all comments are evil? No. I just hate wasteful comments that could be eliminated completely if the code made sense.

Link to comment
Share on other sites

  • 0

Think about it. You are a programmer. You are running from the police. You get shot. You are bleeding a lot. Oh my God! They don't know your blood type! It's a public hospital, they don't have the right substances! You have nothing but... your pen drive! And luckily you type your blood type every time!

Anyway, personally, I think every file must have a header. Comments are very important, but you don't need to put the header changing only the "modified" and stuff every time you modify the code. I think it's a much better approach to save different copies of the file in a SVN/CVS repository every time you do a major change.

Link to comment
Share on other sites

  • 0
Think about it. You are a programmer. You are running from the police. You get shot. You are bleeding a lot. Oh my God! They don't know your blood type! It's a public hospital, they don't have the right substances! You have nothing but... your pen drive! And luckily you type your blood type every time!
ROTFL! Wow, that makes my day, thanks. I SO wasn't expecting that !
Link to comment
Share on other sites

  • 0
Clean Code (http://www.amazon.com/Clean-Code-Handbook-...p/dp/0132350882) is another good one. The author wrote an entire chapter dedicated to code comments.
I'm reading that right now and I've a hard time with some of his recommendations. Refactoring 30-lines methods into 10 very small ones ?, just so that you can spell out everything your code does in method names? I'm not immediately sold on the idea, honestly. Each method is a lot of scaffolding: there's at least the declaration, often a return statement, probably some minimal documentation, and maybe a unit test or two... Sure, long methods are evil but I prefer Steve McConnell's less extreme take on the issue, in Code Complete.
Anyone who codes and deals with software construction should seriously consider reading Code Complete 2. Whatever your style, whatever language you code in, whatever your skill level is.
Agreed. First programming book I ever read, and the best to date.
Link to comment
Share on other sites

  • 0
I'm reading that right now and I've a hard time with some of his recommendations. Refactoring 30-lines methods into 10 very small ones ?, just so that you can spell out everything your code does in method names? I'm not immediately sold on the idea, honestly. Each method is a lot of scaffolding: there's at least the declaration, often a return statement, probably some minimal documentation, and maybe a unit test or two... Sure, long methods are evil but I prefer Steve McConnell's less extreme take on the issue, in Code Complete.

I disagree. Most of the work can be eliminated by a good IDE. The tests would depend on what you're trying to accomplish. One could argue that you would need the tests regardless of how the code is actually broken down.

Link to comment
Share on other sites

This topic is now closed to further replies.
  • Recently Browsing   0 members

    • No registered users viewing this page.