java check my comments

[ultimate99]

Can you please check my comments in this code?

Do I need to add more, be more specific?

What about JavaDoc, do I need any?

THanks!

TTT.zip

[+Nik Louch Subscriber²]

Not downloading. But comment as little or as much as you need, surely?

[still1]

The comments are ok but i have some suggestions I wouldn't comment that explains what the code is doing literally.

--Wouldn't use these--

//Increment by 1

//When count reaches 9 or above, check for winner -- this one is ok but its better to make it more generic

--Would use more like this--

//Check for placed "X" on buttons

//Display output if "X" wins

I would comment on modules or block of functionality rather than literally commenting what the code is doing

[Singh400]

Might be easier if you upload the code to PasteBin or use the

 tags here.

[+Majesticmerc MVC]

The golden rule of comments: The code should describe WHAT is happening, the comments should explain WHY its happening.

If you have to write a comment explaining what a piece of code is doing, you should be re-writing it to make it more obvious. Comments like "// Increment by 1" are redundant, as I could very easily look at the code to figure that out. On the other hand, if a piece of code is doing something, but it's not immediately obvious WHY it's doing it, just add a brief comment to explain why the code is the way it is (for example, if you have to write a hack to work around a bug in someone elses code, explain why the hack is there).

[ultimate99]

THanks for the comments so far.

Here's pastebin: http://pastebin.com/fB05bkt9

[-Alex-]

Link is broken!

[firey]

I comment the code in a way anyone reading it (that knows the language or not) understands it. My comments are almost pseudo code. So I do things like:

//Okay, first we need to build our query

string q = "SELECT..........";

//Now, we need the result to give us the items

string result = clsMain.Database.dbQuery(q);

//Now split it up to get all the parts

string[] split = result.Split(';');

//And finally loop through the results

foreach (string s in split)

if (s != null)

blah;

else

break;

//Now we move to getting the users....

[vhane]

I comment the code in a way anyone reading it (that knows the language or not) understands it. My comments are almost pseudo code. So I do things like:

//Okay, first we need to build our query

string q = "SELECT..........";

//Now, we need the result to give us the items

string result = clsMain.Database.dbQuery(q);

//Now split it up to get all the parts

string[] split = result.Split(';');

//And finally loop through the results

foreach (string s in split)

if (s != null)

blah;

else

break;

//Now we move to getting the users....

Your comments shouldn't read like pseudo code. I wouldn't cater for people who aren't familiar with the language/framework. By over-commenting you are making the code harder to read for people who actually know the language/framework. The latter should be your audience since they are the ones who are most likely to be reading your code and have a use for it in the first place.

Do not describe what the code is doing. Just document why you are doing what you are doing.

You should also pick better variable names. Once you do that you'll find that the above code doesn't need to be commented at all. So, instead of saying something like:

//Okay, first we need to build our query
string q = "SELECT..........";[/CODE]

You'd do:

[CODE]string query = "SELECT..........";[/CODE]

Also, if you find yourself having to split strings that you've just grabbed from your database, you should consider moving that data out into another table.

[firey]

Your comments shouldn't read like pseudo code. I wouldn't cater for people who aren't familiar with the language/framework. By over-commenting you are making the code harder to read for people who actually know the language/framework. The latter should be your audience since they are the ones who are most likely to be reading your code and have a use for it in the first place.

Do not describe what the code is doing. Just document why you are doing what you are doing.

You should also pick better variable names. Once you do that you'll find that the above code doesn't need to be commented at all. So, instead of saying something like:

//Okay, first we need to build our query
string q = "SELECT..........";[/CODE]

You'd do:

[CODE]string query = "SELECT..........";[/CODE]

Also, if you find yourself having to split strings that you've just grabbed from your database, you should consider moving that data out into another table.

Trust me, I go through code that other people write, the last thing I want to do is figure out what the hell they were doing. I would rather just read the comment and know what they were thinking when they wrote it. There are a million and one ways to write a set of code. I'd rather not have to run through the code in my head (even though I know the language). So I do what works for me.

Also, when I pull date from a database I often pull multiple fields. I then split it up as not all the fields will be used for one thing.

For example.. I may pull a userID, userFirstName, userLastName, userEmail, userDOB. I then split it up and fill in the appropriate input fields with the data. I don't use Datasets, I don't use gridviews, I don't use built in database connections.

[vhane]

Trust me, I go through code that other people write, the last thing I want to do is figure out what the hell they were doing. I would rather just read the comment and know what they were thinking when they wrote it. There are a million and one ways to write a set of code. I'd rather not have to run through the code in my head (even though I know the language). So I do what works for me.

You can figure out what the code is doing by reading the code itself. Trust me, it becomes second nature after a while and you won't need to translate the code to english in your head.

The thing with comments is that you can only trust them so far. Comments have to be maintained to and they can get outdated. At the end of the day, the code is what gets executed.

Don't over-comment. Don't repeat yourself.

[Brian M. Veteran]

I always stick to javadoc conventions, and attach comments to the end of lines if needed. I also tend to use very verbose method names (which I guess is an overhang since my primary language is Objective-c, but it makes it much easier to understand your code later on, and makes life much easier for anybody who needs to change/refactor things).

/**
* Brief description of what the method does. Followed by a
* short description of why it does something, or how it does it
* (if it's a weird way/important/nice to know).
*
* @param   x   Description of variable x
* @param   y   Description of variable y
* @return   What the method returns
* @throws  IOException   Reason for throwing an IOException
*/
public float calculateAreaOfCube(int x, int y) throws IOException
{
   return (x*y);
}
[/CODE]

[sbauer]

I comment the code in a way anyone reading it (that knows the language or not) understands it. My comments are almost pseudo code. So I do things like:

//Okay, first we need to build our query

string q = "SELECT..........";

//Now, we need the result to give us the items

string result = clsMain.Database.dbQuery(q);

//Now split it up to get all the parts

string[] split = result.Split(';');

//And finally loop through the results

foreach (string s in split)

if (s != null)

blah;

else

break;

//Now we move to getting the users....

I read through a lot of code as well and these comments would annoy me. They're not adding anything at all. clsMain bothers me as well.

//Okay, first we need to build our query
string q = "SELECT..........";

Like another person said you could just name the variable query and remove the comment.

//Now, we need the result to give us the items
string result = clsMain.Database.dbQuery(q);

What does the result represent? Also, dbQuery method is pretty redundant. You're in the context of the database already so the db in front of query isn't needed.

//Now split it up to get all the parts
string[] split = result.Split(';');

Your comment is telling us that we're splitting the result into "parts". Well yeah, the code tells us that much. Actually, the code tells us more. What are the parts? Again, what does the original result represent? Meaningful variable names would go a long way. You could also refactor this into a new method with a descriptive name. The comment just isn't needed.

//And finally loop through the results
foreach (string s in split)
if (s != null)
blah;
else
break;

The comment isn't very useful. The foreach is a loop so we know we're looping. The variable names leave a lot to be desired so it's hard to guess what this is trying to do.

You could refactor this into a method as well and give it a descriptive name.

[firey]

I read through a lot of code as well and these comments would annoy me. They're not adding anything at all. clsMain bothers me as well.

//Okay, first we need to build our query
string q = "SELECT..........";

Like another person said you could just name the variable query and remove the comment.

//Now, we need the result to give us the items
string result = clsMain.Database.dbQuery(q);

What does the result represent? Also, dbQuery method is pretty redundant. You're in the context of the database already so the db in front of query isn't needed.

//Now split it up to get all the parts
string[] split = result.Split(';');

Your comment is telling us that we're splitting the result into "parts". Well yeah, the code tells us that much. Actually, the code tells us more. What are the parts? Again, what does the original result represent? Meaningful variable names would go a long way. You could also refactor this into a new method with a descriptive name. The comment just isn't needed.

//And finally loop through the results
foreach (string s in split)
if (s != null)
blah;
else
break;

The comment isn't very useful. The foreach is a loop so we know we're looping. The variable names leave a lot to be desired so it's hard to guess what this is trying to do.

You could refactor this into a method as well and give it a descriptive name.

As I described above.. my comments are like pesudo code. They explain my line of thinking and what I was doing when I wrote the code.

It's almost like a plain english version of the code. I try to reduce code as much as I can (methods only when required, non-temp variables when required, etc). I also name my things not based on the end context, but as it is now.

For example.. I call all my classes "cls[Name]" even though I could just do it as "Name". That's just how I program.

And the "clsMain" well it's the Main Class, and such it is named that.

[sbauer]

As I described above.. my comments are like pesudo code. They explain my line of thinking and what I was doing when I wrote the code.

It's almost like a plain english version of the code. I try to reduce code as much as I can (methods only when required, non-temp variables when required, etc). I also name my things not based on the end context, but as it is now.

I know. I read your original post. I just disagree with it. I just don't think "that's just how I program" is a good reason for doing anything. Anybody could say the same thing and we would never improve on anything. In 97 I was writing Perl with flat files. The code was awful and needed improvement. I could have said, "Well, this is just how I program" and dismissed improvement opportunities.

For example.. I call all my classes "cls[Name]" even though I could just do it as "Name". That's just how I program.

But why? Why do you do that?

And the "clsMain" well it's the Main Class, and such it is named that.

What's a main class? Are we talking about your program's entry point or is it a god object?

Everyone should really check out the book "Clean Code" by Robert Martin. I think it should be required reading for everyone.

[firey]

But why? Why do you do that?

Because it's how I was taught to name things, and I carried it on, and in my head when I am coding to me it is easier and cleaner. I know what to type when I want a class, I know what's what based on the start of the name.

What's a main class? Are we talking about your program's entry point or is it a god object?

Main entry point, the main class in which everything is initialized and set, where some global references are stored.

Also, the code in which the above would be used is inside of a .DLL File which follows an addon/module system I wrote to allow DLL files to be loaded dynamically by the parent program to add functionality (think of them like addons for WoW).

However the Addons can talk to each other without any direct references so the clsMain also holds the communication events and such necessary for this. It is also the location that the initial calls for initializing are made via the base program.

Everyone should really check out the book "Clean Code" by Robert Martin. I think it should be required reading for everyone.

I'll give it a look.

[simplezz]

Good code should be self documenting. And whatever you do, don't use hungarian notation in any form. If you're modelling real world objects, give your classes/structures the same names. It makes for much easier reading and maintenance.

[Brony]

I always stick to javadoc conventions, and attach comments to the end of lines if needed. I also tend to use very verbose method names (which I guess is an overhang since my primary language is Objective-c, but it makes it much easier to understand your code later on, and makes life much easier for anybody who needs to change/refactor things).

/**
* Brief description of what the method does. Followed by a
* short description of why it does something, or how it does it
* (if it's a weird way/important/nice to know).
*
* @param   x   Description of variable x
* @param   y   Description of variable y
* @return   What the method returns
* @throws  IOException   Reason for throwing an IOException
*/
public float calculateAreaOfCube(int x, int y) throws IOException
{
   return (x*y);
}
[/CODE]

I agree.

Commens should indicate how every function/member works. It is not quite needing to comment each part of the code, unless it requiress a special logic or it is too tricky (dragon-lies-here code for example)