The Old Joel on Software Forum: Part 3 (of 5)

Need a new compile error...

The .Net compiler will issue a warning if there is code that will never run - "This code is unreachable" or something to that effect.

I'd sure like a warning for "This code won't run when you think it will"...

Just venting, thanks. :-)

Philo

Philo
Thursday, October 9, 2003

I know the feeling. I did this the other day:

class SomeClass
{
string eventName;

public SomeClass( string eventType ) {
this.eventName = eventName;
}
}

See the bug there? I didn't, until I tested it against somebody elses stuff and got very embarassed.

A "code has no side-effect" warning would have really helped right there.

Chris Tavares
Thursday, October 9, 2003

The Microsoft (managed) C++ compiler has an annoying behaviour:

If you have:
virtual int Foo()
{
throw new Exception("unimplemented");
}

You get an error:
"Foo must return a value"

virtual int Foo()
{
throw new Exception("unimplemented");

return 0;
}

You get a warning:
"'return 0' unreachable code"

Since I compile with warnings as errors it makes for an impasse!

I end up with something ugly:

virtual int Foo()
{
if (this != NULL) throw new Exception("unimplemented");

return 0;
}

(errors messages all paraphrased).

Rob Walker
Thursday, October 9, 2003

The sick, sick thing about Chris' example is that in C++, thanks to copy constructors, that code could easily have lots of unintended side effects.

One way to coerce C++ into emulating try { ... } finally { ... } is to instantiate a variable on the stack that has something interesting in its destructor. You never do anything with the variable, you just want to be sure its destructo gets called when that block of code is exited for any reason.

Sick, sick, sick...

Reginald Braithwaite-Lee
Thursday, October 9, 2003

Reginald,

I have to object to your characterization "One way to coerce C++ into emulating try { ... } finally { ... } is to instantiate a variable on the stack that has something interesting in its destructor".

I would instead say that "one way to get around your language not having destructors which are called at predictable times is to introduce a 'finally' keyword to do the same thing (only not as well, since you can't guarantee that multiple try blocks all do the appropriate thing in their finally)".

Exception guy
Thursday, October 9, 2003

Chris - The way I avoid exactly that mistake is to systematically name constructor arguments which init some member, based on the name of the member that it will initialize. In C++, I would write:

class SomeClass
{
public:
SomeClass(const string& eventName ) {
eventName_ = eventName;
}
private:
string eventName_;
}

(The 'trailing-underscore-means-member' convention both makes it easy to recognize members, and gives an obvious name for corresponding ctor args - just remove the underscore)

Exception guy
Thursday, October 9, 2003

Exception guy: we are in violent agreement.

Hereafter I shall say:

One way to get "your hotch pot of design compromises otherwise known as a programming language which happens to be blessed by Fortune500 corporate IT departments" to do something that is easy and sensible to do in Lisp, the One True Programming Paradigm, is to...

Reginald Braithwaite-Lee
Thursday, October 9, 2003

add pragma to disable those warnings that you don't want to see because you know that those are ok warning.

na
Thursday, October 9, 2003

My favorite (C++):

class Foo: public Bar {
int blop;

Foo(Gnork a, int blop): Bar(a), blop(blop) {}
Foo(const Foo &other): Bar(other), blop(blop) {}
};

In the first constructor, the argument "blop" refers to the parameter in the constructor.

In the second (copy) constructor, the argument "blop" refers to this->blop when we meant other.blop.

I spend a _long_ time tracing through this one before I found it...

David Jones
Thursday, October 9, 2003

I'm firmly on the side of Exception guy, and it's because in C++ you only have to write the cleanup code once.

Consider:

class MyAutoCriticalSection
{
CriticalSection& cs_;
public:
MyAutoCriticalSection(CriticalSection& cs) : cs_(cs) {cs.lock();}
~MyAutoCriticalSection() {cs.unlock();}
}

Then you have:

{
MyAutoCriticalSection(cs);
// blah
}

versus:

try
{
cs.lock();
// blah
}
finally
{
cs.unlock();
}

EVERY time you use the cs, you have to write the finally block. That's tedious and error-prone. If you use the auto-cs, you only have to write it once and forget about it.

You can do this in .NET too with IDisposable and the 'using' keyword, but I don't find the syntax as nice ... maybe it's just familiarity though...

Michael Kale
Thursday, October 9, 2003

Whoops, that should be:

{
MyAutoCriticalSection(cs) foo;
// blah
}

But you guys get the idea....

Michael Kale
Thursday, October 9, 2003

Regarding the confusion over what the C++ constructor initializer list is initializing or the earlier misnamed argument problem (I'm assuming that was C# code), this is *exactly* the reason why conventions such as m_ exist. People like to whine about Hungarian notation but putting a simple little prefix on member variables eliminates problems like these. Member variables are fundamentally different from local variables and should be named in a matter that makes this obvious.

SomeBody
Thursday, October 9, 2003

Yes, my original example was C# code.

The actual cause of the bug was that I was renaming the variable, and missed a spot.

As far as m_, xx_, etc., I did that for years with C++, and I just don't like it. If anything, I think the parameters should get the warts, not the member variables, since the member variables are used a lot more.

I would be happy if C# adopted python's rule: no implicit this (although Python calls it self). If you want to access a member variable, say "this.xxx". Simple, no warts required, explicit, and obvious.

Of course, then everyone would say "whaddaya mean I've gotta type five extra characters every time I want a member variable?"

Chris Tavares
Thursday, October 9, 2003

Chris,

The Java IDE that I use highlights that as a "Silly Assignment" :)

Rhys Keepence
Thursday, October 9, 2003

I use parameters a lot more than member variables.

Parameters are your friends once you realise that member variables are basically just global variables with a more limited scope. Sharing state between all the methods of your class is a necessary evil, you want to cut down on it.

Hence, parameters should be what you';re working with most of the time - you want to be passing stuff around, not sitting around where multiple methods could touch it and a change to some other method could break yours.

Prefixes are damn ugly, and _ is hard to type though. I agree with the this.blah/self.blah idea ... then again, Python gets most usability stuff like that done well.

Sum Dum Gai
Thursday, October 9, 2003

The convension of Symbian OS is to prefix all arguments with a and all member (instance) variables with i.

void Class::Function(int aFoo)
{
iFoo = aFoo;
}

I find this style to be very clean and consistent (as the example above partly demonstrates).

In PHP, because all member variables are public, it's common convension to prefix members intended to be private with a single underscore. It's also manditory to use the $this reference in order to access members in functions. This seems to work well, in most cases.

Food for thought, anyway.

Almost Anonymous
Friday, October 10, 2003

There are numerous conventions for naming member variables:
_member
myMember
itsMember
member // camel case
member_

I have used all of them. The first seems to be common in C++ and the .Net Framework. The last has the benefit it also work in Classic VB, but use with care: The separator line in the IDE between sections may hide the underscore.

The "itsMember" version doesn't feel natural to me, so I don't use it anymore.

I am currently using "_member", but I think my code looks less cleaner than it used to. I will try some more, but I suspect "myMember" is a winner.

My comment on >>Sharing state between all the methods of your class is a necessary evil, you want to cut down on it.<<

I think this is getting it backwards. You don't cut down on members, you add them to avoid duplication. If many private methods share the same parameter, then replace it with a member. Privates have usually no business of changing members. This rule applies to publics as well when the parameter never change, but can't be a const.

Thomas Eyde
Friday, October 10, 2003

Should one prefix static memeber variables with SM? Just wondering.

Friday, October 10, 2003

In Java I prefix static member variables with the class name.

John Topley (www.johntopley.com)
Friday, October 10, 2003

Using '_' before a variable name is usually not a good idea. Both C/C++ reserve symbols starting with '_' or '__' (double underscore) to be used by the standart libraries/compiler symbols, so the guy reading your code may guess wrong from where '_member' is from...

Personaly I used myMember for sometime, but now I think that m_Member is easier to read and faster to notice.

whatever
Friday, October 10, 2003

I find m_ too hard to type. And it's ugly, too.

Thomas Eyde
Sunday, October 12, 2003

Considering protected member fields, underscores and camel casing are out as options for naming member fields for these reasons:

_ or __ for protected member fields is not CLS-compliant, thus this is not a good standard for member fields.

Camel casing is out as well, since protected fields and properties differing only by the capitalization of the first character (e.g. protected member foo versus public property Foo) are also not CLS-compliant.

Therefore, m_, in all its ugliness is the only viable standard (other than a non-standard suffix or prefix which by definition is not a standard) for naming member fields, assuming you have a need for protected member fields. Arguably you could avoid using protected member fields and use protected properties instead, following property naming conventions (Pascal Casing). In this case, camel case and _ prefixes are only private and avoid the CLS complaint issue.

RC
Wednesday, June 30, 2004