View Single Post
  #5   Spotlight this post!  
Unread 01-25-2011, 11:44 PM
basicxman basicxman is offline
Emily Horsman
FRC #2200 (MMRambotics)
Team Role: Programmer
 
Join Date: Oct 2007
Rookie Year: 2007
Location: Burlington, Ontario
Posts: 971
basicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant futurebasicxman has a brilliant future
Send a message via AIM to basicxman Send a message via MSN to basicxman Send a message via Yahoo to basicxman
Re: Bad Programming Practices

The following is my entirely unstructured views on code commenting and self documentation based on several years of professional programming, freelance, robotics, and just mucking around with code.

Quote:
Originally Posted by wdell View Post
I go back to mine during the day, clean it up and add additional comments where necessary.
I believe additional refactoring should always be done after the initial code is done. I often try to get people who have never read code in their life to see how much of my program they can understand.

Commenting at this stage is especially useful because the concepts are still fresh in your mind, but you get a little more perspective on what you might not remember/understand a month down the road.


Quote:
Originally Posted by dbeckwith View Post
I just never get around to adding comments because since it's my code, it's obvious to me what it says
Ask yourself this in two months.


Quote:
Originally Posted by lineskier View Post
Every function (except getters and setters) should be commented.
I try to make a rule that every function and class should have a Javadoc style comment with a description and if in anyway non-obvious - describing parameters and return values.


Quote:
Originally Posted by wdell View Post
I'm noticing a disturbing trend here. While the code being shared is great, the actual programming isn't.
While very helpful as an overview, I've found most of the example code that comes with standard installations are very messy. Agreed.

Quote:
Originally Posted by wdell View Post
I've been in college courses where it was a requirement to comment every single line.
This is dumb, it's not only a waste of time but breaks the golden rule of commenting. You end up seeing things like this:

Code:
++i; // Increment i by one.
Frankly, it makes the code way harder to read - especially with syntax highlighting. I know some programmer's who set comments to the same colour as their background colour; while I find this a bit extreme I quite frequently see their point.

Folding is also a great alternative to this, but it's hard to find a good folding plugin for most editors in my opinion.


First to state the commonly accepted rule of commenting (I like to call it the golden rule) for those who have never heard about it before:
Quote:
Comment why you did it, not what it does.

If you have to comment what it does, you should be making better use of two awesome concepts called encapsulation and abstraction. For instance, would you prefer to see:

Code:
// ...
float processedEncoderValue = GetEncoder() * scalingCoefficient - offset; // Process the raw encoder value.
// ...
or

Code:
float GetAndProcessEncoderValue() {
  return GetEncoder() * scalingCoefficient - offset;
}
// ...
float processedEncoderValue = GetAndProcessEncoderValue();
// ...
This benefits in two ways, self-documented code and makes the program more modular and organized. You can put functions in another source file, you can't put those comments in a difference source file.

Properly naming variables also heavily helps self-documented code making things clean and prevents end-of-line comments that not only slow down development time but add a lot of fluff to the code.

Using constants everywhere is something I treasure along the same lines as properly naming variables. If you ever have a value that somebody will need to check a comment elsewhere to see what it means, it should be in a constant with a human-readable name. For instance,

Code:
MyRobot():
  pan(3), // Pan servo PWM port #3
  tilt(4),  // Tilt servo PWM port #4
  searchSwitch(1), // Digital I/O search toggle switch port #1
  cameraLED(2), // Digital I/O camera LED port #2 
  joystick(1) // Left joystick USB port #1
{ // ... 
}
I would have as
Code:
MyRobot():
  pan(PAN_SERVO_PWM_PORT),
  tilt(TILT_SERVO_PWM_PORT),
  searchSwitch(SEARCH_TOGGLE_SWITCH_PORT),
  cameraLED(CAMERA_LED_PORT),
  joystick(LEFT_JOYSTICK_PORT)
{ // ... 
}
I frequently find myself, and others, guilty of doing things like this:
Code:
// Feed watchdog.
FeedWatchdog();

// Declare variables to store text.
char text[255];
char text2[255];
char text3[255];
char text4[255];

// Format and fill text.
sprintf(text, "X: %d  Y: %d", currentParticle.center_mass_x, currentParticle.center_mass_y);
sprintf(text2, "Particle:Image %.4f", currentParticle.particleToImagePercent);
sprintf(text3, "Iteration: %d", currentIterationCycle);
sprintf(text4, "Num Iters: %d", numberOfIterations);

// Clear Driver Station.
driverStation->Clear();

// Print to the Driver Station.
driverStation->PrintfLine(DriverStationLCD::kUser_Line1, text);
driverStation->PrintfLine(DriverStationLCD::kUser_Line2, text2);
driverStation->PrintfLine(DriverStationLCD::kUser_Line3, text3);
driverStation->PrintfLine(DriverStationLCD::kUser_Line4, text4);

// Update the Driver Station
driverStation->UpdateLCD();
Ask yourself, are any of these really necessary?

I'm sure you could find a million examples on Google of bad code commenting, but the best way to adopt a good code commenting style (and general code style, i.e. formatting, etc) is reading other's code.

I'd like to raise another point on the formatting of your comments, when you do write proper comments, remember other human beings are reading them, and most likely reading them in English. It's my personal preference, but I find it annoying when I see this (it takes your clean looking, professional code and makes it look...well, immature):

Code:
//necessary for proper calculations due to...
Comments are English text. They should start with a capital and should not be against punctuation (//).

Code:
// Necessary for proper calculations due to...

I generally only use comments for the following situations in robot code:
  • Header comment (author, description)
  • Explaining mathematical operations
  • Javadoc-style documentation for classes and functions
  • Organizing constants

I can't stress enough: Place a comment to explain why it does it rather than what it's doing.

Now of course, we don't need to explain why something needs to be done due to explicit idiosyncrasies of a language or paradigm, for instance the following should never occur:

Code:
delete image; // We need to free up memory
However the following might be acceptable under certain conditions:

Code:
delete image; // At this point, the system is running out of memory and must sacrifice this image for more essential data.
One last point on TODO, FIX LATER comments. I am not totally against this, grepping through your programs for TODO does not make me want to hurt kittens. However I really feel like these sorts of comments are what version control and other tools are for.

That's my two cents!

Here's some reading material if I wasn't boring enough:
Jakob Kulzer - My Case Against Code Comments
Comments vs. Self Documenting Code

If you are looking for an entire paradigm shift, I'd have to recommend getting involved with the Ruby community - Rubyists heavily stress modular, neat, beautiful code with emphasis on refactoring and human-readable code.

Here is an example of a Ruby Domain-Specific-Language code snippet, bow before it.

Code:
describe Account do
    context "transfering money" do
      it "deposits transfer amount to the other account" do
        source = Account.new(50, :USD)
        target = mock('target account')
        target.should_receive(:deposit).with(Money.new(5, :USD))
        source.transfer(5, :USD).to(target)
      end

      it "reduces its balance by the transfer amount" do
        source = Account.new(50, :USD)
        target = stub('target account')
        source.transfer(5, :USD).to(target)
        source.balance.should == Money.new(45, :USD)
      end
    end
  end
I'm not really sure how any comments could improve that code.

EDIT: I forgot the paragraph about TODO style comments.

Last edited by basicxman : 01-25-2011 at 11:53 PM.
Reply With Quote