What Are the Hallmarks of Good / Bad Code?

[IronWing]

What are you talking about?

Blackjack

 

[Gryz]

Good (possibly, depending on context): a=x // Store the x-position in the accumulator.

A much better way would be to give your variables good names.

int accumulator;
int x-position;

accumulater = x-position;

Now you don't need to comment that line. And it is suddenly more readable than any comment could possibly be.

For some reason, a majority of programmers thinks that they need to choose the shortest variable-name they can come up with. Wrong. Pick descriptive names. 20+ characters long if you have to. Do this for variables, functions, constants, everything.

 

[purbeast0]

A much better way would be to give your variables good names.

int accumulator;
int x-position;

accumulater = x-position;

Now you don't need to comment that line. And it is suddenly more readable than any comment could possibly be.

For some reason, a majority of programmers thinks that they need to choose the shortest variable-name they can come up with. Wrong. Pick descriptive names. 20+ characters long if you have to. Do this for variables, functions, constants, everything.

what really grinds my gears is when people leave vowels out of variables to save like 3-4 characters.

goodService
badSvc

hate code like that. spell out service ffs.

 

[Gryz]

this could easily just be written in code though, no need to look at comments to the right of all your code.

drawTwoCards();

if (sumOfFirstTwoCardsEquals(10)) {
  splitTheHand();
}
if (sumOfCardsEquals(11)) {
  doubleDown();
}
if (sumOfCardsLessThan(17) {
  drawAnotherCard();
}

comments simply aren't necessary if you have code written that way.

Don't like it.
If I had to do code-review before commit, your code would not pass.

1) You use raw numbers in your code. That's an absolute no-no. Use constants. This makes it easier to understand where those numbers come from. And it makes it easier if you want to change a constant later, and you don't have to search for hundreds of occurences of the string "10".

2) The functionnames indicate what each step does. But your program does not indicate at all what it is trying to do. I'm not a cards player, I have no clue at all what problem this is supposed to solve. Or what algorithm is being used.

Why do I split when I get a sum of 10 ? What is "double down" ? What's special about the numbers 11 and 17 ? And again, what is this ? A nice block comment above this code could be a nice explanation.

 

[purbeast0]

Don't like it.
If I had to do code-review before commit, your code would not pass.

1) You use raw numbers in your code. That's an absolute no-no. Use constants. This makes it easier to understand where those numbers come from. And it makes it easier if you want to change a constant later, and you don't have to search for hundreds of occurences of the string "10".

2) The functionnames indicate what each step does. But your program does not indicate at all what it is trying to do. I'm not a cards player, I have no clue at all what problem this is supposed to solve. Or what algorithm is being used.

Why do I split when I get a sum of 10 ? What is "double down" ? What's special about the numbers 11 and 17 ? And again, what is this ? A nice block comment above this code could be a nice explanation.

yeah we already went over block comments going above functions being okay. if you weren't sure what the function double down does, you could go to the function declaration to find out.

and i agree about the constants. if it was a real application, i would definitely have constants.

 

[Spungo]

For some reason, a majority of programmers thinks that they need to choose the shortest variable-name they can come up with. Wrong. Pick descriptive names. 20+ characters long if you have to. Do this for variables, functions, constants, everything.

After watching youtube videos about C#, I've come to 2 conclusions:
1) 90% of all C# programmers are Indian
2) Maybe 50% of C# programmers suck at typing.

It's frustrating to watch a guy make several mistakes when typing something like "Regex" or "Console.WriteLine".

 

[smackababy]

yeah we already went over block comments going above functions being okay. if you weren't sure what the function double down does, you shouldn't be using it!

FTFY


There are fancy comments in Java (not sure about other languages) that support the mouse over description.

 

[Spungo]

Don't like it.
If I had to do code-review before commit, your code would not pass.

1) You use raw numbers in your code. That's an absolute no-no. Use constants.

const int seventeen = 17;
if (GetSum() >= seventeen)
{
   Stay()
}
else
{
   DrawCard();
}

 

[Ken g6]

const int seventeen = 17;
if (GetSum() >= seventeen)
{
   Stay()
}
else
{
   DrawCard();
}

I'm going to have to amend that to "Comment everything intelligently." Or "Comment everything descriptively."

I'm going to have to amend the constant rule too, to, let's say, "Use descriptively-named constants".

Oh, also, it's somewhat of a standard to make constants ALL CAPS.

IT'S ALSO DIFFICULT TO READ THINGS WRITTEN IN ALL CAPS

I find them perfectly readable as long as they're not allmashedtogetherlikethis. Use _'s or -'s.

 

[Crusty]

IT'S ALSO DIFFICULT TO READ THINGS WRITTEN IN ALL CAPS

That's the accepted standard for shell and environment variables.

Granted a language like bash isn't that pleasant to read to start with, but one persons opinions are likely based around a specific language and their experiences with it.

I've worked on enough projects with enough people to know that everybody has a different preference, as also evident in this thread. The best thing you can do as a programmer working on a team is adopt a set of practices that everybody uses regardless of having 100% agreement on the standard.

 

[smackababy]

That's the accepted standard for shell and environment variables.

Granted a language like bash isn't that pleasant to read to start with, but one persons opinions are likely based around a specific language and their experiences with it.

I've worked on enough projects with enough people to know that everybody has a different preference, as also evident in this thread. The best thing you can do as a programmer working on a team is adopt a set of practices that everybody uses regardless of having 100% agreement on the standard.

Or, work for a company that has a standard they require you to conform to, along with files that will auto format you code to meet them.

The debate over should the '{' should go at the end of the line or the line below will never be settled amongst the programmers. But, the lead can make a decision and force it upon everyone!

 

[Spungo]

That's the accepted standard for shell and environment variables.

I think we all agree that all caps is difficult to read. This is the reason the forum never forces people to capitalize things, but the forum will automatically edit your post if you post in all caps.

The best thing you can do as a programmer working on a team is adopt a set of practices that everybody uses regardless of having 100% agreement on the standard.

:thumbsup:

The debate over should the '{' should go at the end of the line or the line below will never be settled amongst the programmers. But, the lead can make a decision and force it upon everyone!

It's forced if you use Visual Studio.
I think a lot of it depends on the culture of the language. Perl tries to be as tight as possible, so you'll find a higher than average number of people putting the {} on the same line as other stuff:

if (conditon) {
   stuff;
} else {
   other_stuff;
}

 

[dave_the_nerd]

I think we all agree that all caps is difficult to read. This is the reason the forum never forces people to capitalize things, but the forum will automatically edit your post if you post in all caps.

No. It does that BECAUSE ALL CAPS IN INTERWEBS-ENGLISH IS YELLING. It's rude.

Also harder to read, but primarily it's rude. Like using tab indents instead of spaces or automatically replacing UNIX-style line endings with windows ones when you check code in.

 

[purbeast0]

constants should always be in all caps with a _ between the words imo.

if (getSum() < seventeen)

just looks worse than

if (getSum() < SEVENTEEN)

the ALL_CAPS screams that it is a constant value as well, so you don't have to even think twice about what that variable means.

 

[smackababy]

It's forced if you use Visual Studio.
I think a lot of it depends on the culture of the language. Perl tries to be as tight as possible, so you'll find a higher than average number of people putting the {} on the same line as other stuff:

if (conditon) {
   stuff;
} else {
   other_stuff;
}

I use Visual Studio every day. It isn't forced.

I was talking about:

if (condition){
   stuff;
}

vs.

if (condition)
{
   stuff;
}

And even then, I do a few:

if(condition)
   stuff;
else
   stuff;

But, I do try and limit that, in the event the scope of the statement expands.

 

[reallyscrued]

Posting in a useful thread.

Wish I had my code reviewed by some of you guys
Could use the better habits.

 

[Ken g6]

constants should always be in all caps with a _ between the words imo.

:thumbsup:

I think we all agree that all caps is difficult to read.

WHAT'S SO HARD...

It's rude.

Also harder to read, but primarily it's rude.

Oh, sorry. What's so hard about reading all-caps? You see it every day on signs, advertisements, packaging, etc.

 

[Gryz]

You can all discuss for a full year on what's the best coding style. And still not get a result. Certainly not a result that pleases everyone. And different languages require different coding styles. So when people discuss coding styles in general on the net, there will never be agreement.

The most important thing is that all programmers working on the same project, or working for the same company, should share the exact same coding style. I mean *exact*. It would be best if one of the senior programmers would write a coding-style document, discuss it with half a dozen of his senior colleges, then adjust it. And then make it available to all programmers in the company.

I used to work for a large multinational. That had a code base of over 10 million lines of code. (One piece of software). Mostly C-code. They had a nice little document. Maybe 5 pages or so. It said everything that needs to be said about C coding style. Everybody (a few thousand programmers) used the exact same coding style. That really, really helps.

We had an intern who wrote a tool for us. I didn't mentor him (he was in an office in another continent). When the tool was done, I wanted to see it. And then wanted to see the code. Turns out he wrote it in C++. All company code was in plain C. So that made it almost useless for me. Then I saw the code, and it was the biggest jibberish I had ever seen. I never used that tool. Too bad.

I later joined a startup. Where most of the senior programmers came from that same multinational I just left. One of the first things we did, after designing the overall system, but before writing a single line of code, was write a new C code style document. It was exactly that same style we used in our previous jobs. With maybe 2 or 3 small adjustments. A bunch of other programmers that came from other companies had to adjust. But that took them maybe a week or two. Having one style amongst all your colleagues is so self-evident, I am always surprised when I see discussions about it on the net.

 

[Gronnie]

Or, work for a company that has a standard they require you to conform to, along with files that will auto format you code to meet them.

The debate over should the '{' should go at the end of the line or the line below will never be settled amongst the programmers. But, the lead can make a decision and force it upon everyone!

This!

I don't have a choice about braces, whitespace, tabs, etc.

Asytle for the win!

 

[reallyscrued]

WHAT'S SO HARD...

Oh, sorry. What's so hard about reading all-caps? You see it every day on signs, advertisements, packaging, etc.

Bad example.

It's hard because typical computer fonts are designed to most efficiently use screen space in the scenario of being written in normal casing (lower case with 1 letter uppercase as needed at beginning of sentences). It just looks wrong and squished together if typical fonts are used to write in all caps.

There are different fonts with modified levels of kerning (spacing between letters) for use in scenarios that require all upper case. These typefaces are the ones you see in advertisements and packaging - they are very easy to read and are designed as such.

SPEED LIMIT

See how squished together the word LIMIT looks in all caps with this font? (assuming you left your browser defaults, the I has feet - serif) The L and the I almost look like a rectangular U. Things like that would cause you to read slower or even misread words if an entire paragraph was written in that font in all upper case. Quite annoying.

Compare to this:

 

[Ken g6]

Most editors have the option of setting a different font. Most computer code is also viewed in a monospace font. (Which is (edit: one reason) why people [thread=2069847]should use code tags[/thread]!)

SPEED LIMIT

That has some serifs on the "I" too. But looking over various fonts it seems to be difficult to make one that clearly distinguishes between "I", "l", "1", and "|" without some serifs.

 

[reallyscrued]

Most editors have the option of setting a different font. Most computer code is also viewed in a monospace font. (Which is (edit: one reason) why people [thread=2069847]should use code tags[/thread]!)

SPEED LIMIT

That has some serifs on the "I" too. But looking over various fonts it seems to be difficult to make one that clearly distinguishes between "I", "l", "1", and "|" without some serifs.

All editors I know have the option of setting a different font. But that's not the point.

You insinuated there's no adverse effects to using all caps and gave street signs and packaging as examples. I stated those were bad examples.

Having serifs on the I's, 1's, and l's, like you said, are useful in telling them apart. All caps are only very readable when no serifs are involved or lots of kerning is used - read: not suitable for coding or most computer related typing without taking up lots of screen real estate for no return whatsoever.

Further, not everyone should have to change their default code editor font because someone doesn't understand why all caps is commonly atrocious.

 

[purbeast0]

Having serifs on the I's, 1's, and l's, like you said, are useful in telling them apart. All caps are only very readable when no serifs are involved or lots of kerning is used - read: not suitable for coding or most computer related typing without taking up lots of screen real estate for no return whatsoever.

Further, not everyone should have to change their default code editor font because someone doesn't understand why all caps is commonly atrocious.

 

[Spungo]

I use Visual Studio every day. It isn't forced.

Which version are you using? I'm using 2013 Express. If I type this:

if (condition) {

and hit enter, it automatically turns into this:

if (condition)
{

All of the settings are on default.

 

[Markbnj]

Further, not everyone should have to change their default code editor font because someone doesn't understand why all caps is commonly atrocious.

There are contexts in which all-caps is not only unobjectionable, but increases comprehension. Specifically, case-insensitive languages like SQL. The reason that the upper-case convention was adopted for SQL keywords early on is simply that when the language is not case-sensitive you end up with everyone using their own personal approach, i.e. select, Select, and SELECT.