C++ compiler does not understand comments (yet?)

Bulb

Looking for reasons why it's slow, a colleague of mine found this in our codebase:

	// if the database is the same that was open last time
	if(dbid==revDbIdx)
	{
		// I'll just check whether it's still open
		if(!Revfile.IsOpened())
		{
			// I'll open the file again with original path to database
			if(!Revfile.OpenToRead(database->GetPath(),true)) { RevUnlock(); return false; }
		}
	}
	// it's either different database or I didn't work with any yet
	{
		// this works always
		Revfile.Close(); revDbIdx=0xFFFFFFFF;
		// I'll open the file with original path to database
		if(!Revfile.OpenToRead(database->GetPath(),true)) { RevUnlock(); return false; }
		// I'll store the ID for next read attempt
		revDbIdx=dbid;
	}

The function this is in takes index into array of file names (dbid) and reads something from the corresponding file. It is called hundred times a second, most of the time with the same index as last time and opening a file is fscking expensive on Windows CE (Windows CE filesystem performance is a wtf of it's own). Adding the "else" suggested by the comment in the middle made quite a significant difference. Unfortunately the compiler does not yet interpret the comments.

The comments were in fact in our native language, not English. I translated them, so you can enjoy that guy's commenting style, which is the real wtf here.

Airhead

I wouldn't call this a WTF but an honest error-by-forgetting-to-add-something. And the comments seem to tell pretty much what is happening here so even I understood what he was trying to accomplish.

Evo

I agree. Not much of a WTF. Even though the code could be restructured to be more readable, IMHO. In fact, one comment for this entire block would be enough:

// Open the requested database if it's not already opened.
if(!RevFile.IsOpened() || dbid != revDbIdx) {
  Revfile.close();
  if(!Revfile.OpenToRead(database->GetPath(), true)) {
    RevUnlock();
    revDbIdx = -1;
    return false;
  }
revDbIdx = dbid;

}

The else, I agree, is just a simple mistake. Just like I'll undoubtedly have at least one mistake in the code written above. That doesn't mean it's a WTF.

Xyro

Clearly, TRWTF is that he doesn't cuddle his braces.

} else {

dhromed

@Xyro said:

Clearly, TRWTF is that he doesn't cuddle his braces.
} else {

If you cuddle braces, you misalign the keywords and I dislike that. I used to put { on a newline as well, with the same alignment argument, but I found that the effects on scanability weren't significant, and you gain that extra line.

Abdiel

Must be a new feature of C++0x.

Arenlor

I actually signed up for an account just to reply to this. He forgot to put in an else, and yet because he was commenting everything you were able to find this and add it in. I guess he comments too much for you?

Jonathan

I caused a bug once (that I know of) with an extraneous semicolon like this.

if (condition);

{

doStuff();

}

Even when I was looking directly at it I didn't see the problem until after actually stepping through with a debugger a few times. It would be really nice if there were a compiler warning that caught autonomous braces. I never purposely use autonomous braces so if they exist in my code, it's certainly a bug.

PJH

@Jonathan said:

I caused a bug once (that I know of) with an extraneous semicolon like this.
if (condition);
{
doStuff();
}
Even when I was looking directly at it I didn't see the problem until after actually stepping through with a debugger a few times. It would be really nice if there were a compiler warning that caught autonomous braces.

wtf are 'autonomous braces'? If you're after a warning because of that semicolon, gcc provides one if you need it to:

Paul@DellLaptop /cygdrive/c/temp
$ cat if.c
#include <stdio.h>
void foo(void){
}
int main(void){
        volatile int i=1;
        if(i);
        {
                foo();
        }
        return 0;
}


Paul@DellLaptop /cygdrive/c/temp
$ gcc if.c -o if -Wall -Wextra
if.c: In function `main':
if.c:6: warning: empty body in an if-statement

And braces in the middle of a function without a control statement are useful in C if you want a standards compliant method of declaring variables in a place other than at the top of the function.

morbiuswilters

@PJH said:

wtf are 'autonomous braces'?

Braces that can love. And feel pain.

dhromed

@morbiuswilters said:

Braces that can love. And feel pain.

Braces whose bodies you can rape and pillage.

bjolling

@PJH said:

And braces in the middle of a function without a control statement are useful in C if you want a standards compliant method of declaring variables in a place other than at the top of the function.

Also very useful for doing resource management with RAII

int main(void)
{
   // some initialization code


   // create smaller scope for DataAccess component to live in
   {
      DataAccess dataAccess ("my connection string"); // Constructor opens connections to database

      // do something with dataAccess
      dataAccess.DoSomething("here");

   } // dataAccess goes out of scope. Destructor closes database connections

   // some more code that doesn't need data access

   return 0;
}

The_General

@Bulb said:

The comments were in fact in our native language, not English. I translated them, so you can enjoy that guy's commenting style, which is the real wtf here.

First person singular, perfective? There's your problem right there. To make the compiler understand the comments, you have to use the imperative. Or, depending which version of the compiler you've got, just put

[code]// FASTER!![/code]

(Not sure whether "rychlejší" will work, even with "ty zk.....ec jeden" on the end.)