C++ coding guidelines

Discussion of chess software programming and technical issues.

Moderator: Ras

User avatar
ilari
Posts: 750
Joined: Mon Mar 27, 2006 7:45 pm
Location: Finland

Re: C++ coding guidelines

Post by ilari »

mcostalba wrote:
Sven Schüle wrote: There may be more rules that I don't remember at the moment, but the ones given above are probably the most important ones for me.
One advice that I remenber becuase it is very funny but at the same time very true is: Don't name objects/class that end with 'er'

http://objology.blogspot.it/2011/09/one ... dvice.html
I think that's a fundamentalist approach that may work for dynamic anti-pattern languages like Python but not that well for C++ or Java development. Managers, builders, adapters, decorators, controllers, iterators, wrappers, etc. are well-known and widely used patterns that don't need to be renamed just because the founding fathers of OOP would like that.

I like to use class names that best describe the class, and that means that sometimes the name will be derived from what the class' instances do instead of what they look like. It's intuitive and it's consistent with the way we name things in the real world: we use names that describe functionality like "computer", "dvd player", "project manager" or "transformer". And we also use more abstract names like "car", "sofa", "desk" or "potato". Functionality-based names are a lot easier to invent, and I guess that's why modern things tend to be named that way.
Sven
Posts: 4052
Joined: Thu May 15, 2008 9:57 pm
Location: Berlin, Germany
Full name: Sven Schüle

Re: C++ coding guidelines

Post by Sven »

ilari wrote:
mcostalba wrote:
Sven Schüle wrote: There may be more rules that I don't remember at the moment, but the ones given above are probably the most important ones for me.
One advice that I remenber becuase it is very funny but at the same time very true is: Don't name objects/class that end with 'er'

http://objology.blogspot.it/2011/09/one ... dvice.html
I think that's a fundamentalist approach that may work for dynamic anti-pattern languages like Python but not that well for C++ or Java development. Managers, builders, adapters, decorators, controllers, iterators, wrappers, etc. are well-known and widely used patterns that don't need to be renamed just because the founding fathers of OOP would like that.

I like to use class names that best describe the class, and that means that sometimes the name will be derived from what the class' instances do instead of what they look like. It's intuitive and it's consistent with the way we name things in the real world: we use names that describe functionality like "computer", "dvd player", "project manager" or "transformer". And we also use more abstract names like "car", "sofa", "desk" or "potato". Functionality-based names are a lot easier to invent, and I guess that's why modern things tend to be named that way.
I second that. Programs do not consist of objects being manipulated only but also of objects that manipulate others. There is "material" as well as "tools" in a software system, "passive" as well as "active" objects. Especially the active ones are those that can often be named ending on "er". Or what's wrong with a chess engine that is encapsulating the concept of a "chess player"? Or with a class named "Parser"?

EDIT: o.k., the author of that "Objology" has explicitly listed "Parser" among a few exceptions. But that does not change much.

Sven
Last edited by Sven on Mon Oct 22, 2012 12:44 am, edited 1 time in total.
rreagan
Posts: 102
Joined: Sun Sep 09, 2007 6:32 am

Re: C++ coding guidelines

Post by rreagan »

lucasart wrote:I don't think class encapsulation is relevant in this particular case. Magic bitboards have a functional reality, not an object reality. Basically you have precomputed arrays, and 2 functions (bishop_attack and rook_attack). Encapsulating into a class is conceptually wrong, because an instance of that class doesn't make any sense. Why would I ever want to instantiate several Magic objects ?
That should not be the driving question when deciding on class vs. no-class. Instead, the driving question should be, "Is there an invariant to protect?" See here:

http://www.artima.com/intv/goldilocks3.html

Even if you only have one instance, it still may make very good sense to have a class. Even if the class is tiny (an int wrapper), it may make sense to have a class. For example, in my engine I have a class for Color, PieceType, Piece, Rank, File, Square, and so on. In most engines those are just ints, but there is an invariant to uphold. A Color should be one of three values (white, black, none). If a Color is anything else, I want to know about it, because it's probably a bug. It may even be a buy from somewhere completely unrelated, and a buffer overrun caused a Color to get an invalid value. Using a class makes it easy to check the value before that value gets used. The calling code doesn't have to have to be cluttered with asserts, it's built into the Color class. And now I get to know ASAP when something isn't right in the program. Otherwise that buffer overrun maybe doesn't show up until a bit later, and now it's a lot harder to track down.

If I didn't use classes to wrap these simple types, I would use enums like Stockfish. I preferred to have all of the validity testing built-in and automatic, so I went with classes. All of the validity checking is assert statements, so that goes away in a release build and there is no performance penalty.

Naming convention is not really that important. I wasted a ton of time worrying about it. All that I have as a result of it, is feeling more confident that it's not important :) Okay, if you're working in a large group and sharing code, it's of some importance, but I think there are more important things.

I would prefer to have good naming contexts, rather than worrying about actual names. I (somewhat loosely) follow John Lakos guidelines from his book Large Scale C++ Software Design. Basically he says to think in terms of "components", and a component is a .cpp file and a .h file, usually implementing a single class, along with some test code (unit testing). Even if there is no real class, and you just have a handful of functions, he recommends using a struct with static functions. So at the very least, you have something like this:

Code: Select all

struct Magic {
    static Types::uint64 bishop_attacks (...);
    static Types::uint64 rook_attacks (...);
    static void test ();
}
The benefit of coding like this, which I have appreciated more than any naming convention, is that the context of the code is always 100% clear. If I see some code calling Magic::rook_attacks(), I immediately know that lives in magic.h/magic.cpp.

I also cannot stress strongly enough the importance of unit testing, and the component approach makes that very straightforward. Unit tests are so critical, I can't imagine how I got along before them. They are important to make sure your code does what you think it will do, and probably more importantly, they give you a quick heads up when you make a change that breaks something. Almost all of my components have more testing code than actual implementation code. Here's a simple example:

Code: Select all

void Color::test ()
{
    {
        Color c;
        TEST( !c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::NONE );
        TEST( c.enemy() == Color::NONE );
        TEST( c.to_string() == "-" );
    }
    {
        Color c("w");
        TEST( c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::WHITE );
        TEST( c.enemy() == Color::BLACK );
        TEST( c.to_string() == "w" );
    }
    {
        Color c("b");
        TEST( c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::BLACK );
        TEST( c.enemy() == Color::WHITE );
        TEST( c.to_string() == "b" );
    }
    {
        Color c("W");
        TEST( c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::WHITE );
        TEST( c.enemy() == Color::BLACK );
        TEST( c.to_string() == "w" );
    }
    {
        Color c("B");
        TEST( c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::BLACK );
        TEST( c.enemy() == Color::WHITE );
        TEST( c.to_string() == "b" );
    }
    {
        Color c("banana");
        TEST( !c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::NONE );
        TEST( c.enemy() == Color::NONE );
        TEST( c.to_string() == "-" );
    }
    {
        Color c = Color::WHITE;
        TEST( c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::WHITE );
        TEST( c.enemy() == Color::BLACK );
        TEST( c.to_string() == "w" );
    }
    {
        Color c = Color::BLACK;
        TEST( c.is_color() );
        TEST( c.is_valid() );
        TEST( c == Color::BLACK );
        TEST( c.enemy() == Color::WHITE );
        TEST( c.to_string() == "b" );
    }
}
Color::test() gets called at program startup. TEST() is like an assert macro, except it always gets called, even in the release build. When I run the program, it shows me the test results:

Code: Select all

Component Test Results:
             Success: 3692
             Failure: 0

Success By Component:
           types.cpp: 8
            text.cpp: 26
          random.cpp: 1000
           color.cpp: 40
       piecetype.cpp: 80
           piece.cpp: 162
            rank.cpp: 72
            file.cpp: 104
...
I've only recently started rewriting my engine using this approach, and even though I have far less time these days to spend on chess programming, I get far more accomplished and make more consistent progress. In the past, I would get farther along, and then find some obscure bug, and it would take forever to track down because the offending code was written long ago (with no unit test). Then I'd fix the bug, and that would break something else, and by the time I fixed the bugs, I wasn't having any fun, so I'd take a break from chess programming. With good naming contexts and unit testing, I'm having a lot more fun :)

Overall, coding this way lets you build up something using building blocks. The Lakos book was all about coding in a way that was scalable, and while a chess program is not so big that you run into significant problems scaling, it certainly helps to keep everything organized, and I find I spend a lot less mental energy wondering and worrying if my code is correct. It's certainly not perfect, and I still write buggy code sometimes, but it's a great way to not get lost in the complexity of C++.

I was in the same spot several years ago, feeling lost in the complexity of C++, and the Lakos book helped me tremendously in that area.
mcostalba
Posts: 2684
Joined: Sat Jun 14, 2008 9:17 pm

Re: C++ coding guidelines

Post by mcostalba »

rreagan wrote:Even if there is no real class, and you just have a handful of functions, he recommends using a struct with static functions. So at the very least, you have something like this:

Code: Select all

struct Magic {
    static Types::uint64 bishop_attacks (...);
    static Types::uint64 rook_attacks (...);
    static void test ();
}
The benefit of coding like this, which I have appreciated more than any naming convention, is that the context of the code is always 100% clear. If I see some code calling Magic::rook_attacks(), I immediately know that lives in magic.h/magic.cpp.
Isn't this the case for which namespaces have been invented for?

Anyhow naming conventions and to properly name the stuff you code are 2 different things....chosing a good naming convention is far easier than chosing good names :-)
rreagan
Posts: 102
Joined: Sun Sep 09, 2007 6:32 am

Re: C++ coding guidelines

Post by rreagan »

mcostalba wrote:
rreagan wrote:Even if there is no real class, and you just have a handful of functions, he recommends using a struct with static functions. So at the very least, you have something like this:

Code: Select all

struct Magic {
    static Types::uint64 bishop_attacks (...);
    static Types::uint64 rook_attacks (...);
    static void test ();
}
The benefit of coding like this, which I have appreciated more than any naming convention, is that the context of the code is always 100% clear. If I see some code calling Magic::rook_attacks(), I immediately know that lives in magic.h/magic.cpp.
Isn't this the case for which namespaces have been invented for?
Yes, but as someone else mentioned, there are some differences. With the approach of thinking in terms of components, it's actually not a good thing to split things among multiple files. With a struct, all code in Magic:: is in one of two files (.h/.cpp), which is what I would want. I also think it's advantageous that you can't use "using namespace Magic" and lose the context, and potentially cause more name conflicts.

To me, a namespace is more appropriate for a big library or framework, like std:: or boost::, where it's not reasonable to expect everything should fit into a single pair of .h/.cpp files.
rreagan
Posts: 102
Joined: Sun Sep 09, 2007 6:32 am

Re: C++ coding guidelines

Post by rreagan »

rreagan wrote:
mcostalba wrote:Isn't this the case for which namespaces have been invented for?
Yes, but as someone else mentioned, there are some differences. With the approach of thinking in terms of components, it's actually not a good thing to split things among multiple files. With a struct, all code in Magic:: is in one of two files (.h/.cpp), which is what I would want. I also think it's advantageous that you can't use "using namespace Magic" and lose the context, and potentially cause more name conflicts.

To me, a namespace is more appropriate for a big library or framework, like std:: or boost::, where it's not reasonable to expect everything should fit into a single pair of .h/.cpp files.
But my long-winded point was not to debate these specific details. We could debate this, and where we place our braces, all day long.

My point was, Lucas said the complexity of C++ was causing him problems, and I had felt the same in the past and found a decent solution in Large Scale C++ Software Design. It's not perfect, but it is something you can say, "here, use C++ this way, we tried it, it works, you can build big things and won't get lost in the complexity".
User avatar
Don
Posts: 5106
Joined: Tue Apr 29, 2008 4:27 pm

Re: C++ coding guidelines

Post by Don »

rreagan wrote:
rreagan wrote:
mcostalba wrote:Isn't this the case for which namespaces have been invented for?
Yes, but as someone else mentioned, there are some differences. With the approach of thinking in terms of components, it's actually not a good thing to split things among multiple files. With a struct, all code in Magic:: is in one of two files (.h/.cpp), which is what I would want. I also think it's advantageous that you can't use "using namespace Magic" and lose the context, and potentially cause more name conflicts.

To me, a namespace is more appropriate for a big library or framework, like std:: or boost::, where it's not reasonable to expect everything should fit into a single pair of .h/.cpp files.
But my long-winded point was not to debate these specific details. We could debate this, and where we place our braces, all day long.
You don't use the "one true" brace style?

My point was, Lucas said the complexity of C++ was causing him problems, and I had felt the same in the past and found a decent solution in Large Scale C++ Software Design. It's not perfect, but it is something you can say, "here, use C++ this way, we tried it, it works, you can build big things and won't get lost in the complexity".
Capital punishment would be more effective as a preventive measure if it were administered prior to the crime.
Jan Brouwer
Posts: 201
Joined: Thu Mar 22, 2007 7:12 pm
Location: Netherlands

Re: C++ coding guidelines

Post by Jan Brouwer »

Don wrote:
rreagan wrote:
rreagan wrote:
mcostalba wrote:Isn't this the case for which namespaces have been invented for?
Yes, but as someone else mentioned, there are some differences. With the approach of thinking in terms of components, it's actually not a good thing to split things among multiple files. With a struct, all code in Magic:: is in one of two files (.h/.cpp), which is what I would want. I also think it's advantageous that you can't use "using namespace Magic" and lose the context, and potentially cause more name conflicts.

To me, a namespace is more appropriate for a big library or framework, like std:: or boost::, where it's not reasonable to expect everything should fit into a single pair of .h/.cpp files.
But my long-winded point was not to debate these specific details. We could debate this, and where we place our braces, all day long.
You don't use the "one true" brace style?

My point was, Lucas said the complexity of C++ was causing him problems, and I had felt the same in the past and found a decent solution in Large Scale C++ Software Design. It's not perfect, but it is something you can say, "here, use C++ this way, we tried it, it works, you can build big things and won't get lost in the complexity".
After 20+ years of C/C++ development, I've recently switched to this brace style for my personal projects.
Seems I'm a slow learner :)

I also switched from CamelCaseNames to underscore_names.
Sven
Posts: 4052
Joined: Thu May 15, 2008 9:57 pm
Location: Berlin, Germany
Full name: Sven Schüle

Re: C++ coding guidelines

Post by Sven »

rreagan wrote:My point was, Lucas said the complexity of C++ was causing him problems, and I had felt the same in the past and found a decent solution in Large Scale C++ Software Design. It's not perfect, but it is something you can say, "here, use C++ this way, we tried it, it works, you can build big things and won't get lost in the complexity".
I fully agree. That book of John Lakos was really helpful for me and addresses many typical issues of complexity while also presenting suitable solutions. I also believe everything in that book can, and should, be applied to smaller projects as well, at least to some extent.

Sven
abulmo
Posts: 151
Joined: Thu Nov 12, 2009 6:31 pm

Re: C++ coding guidelines

Post by abulmo »

rreagan wrote:Even if the class is tiny (an int wrapper), it may make sense to have a class. For example, in my engine I have a class for Color, PieceType, Piece, Rank, File, Square, and so on. In most engines those are just ints, but there is an invariant to uphold. A Color should be one of three values (white, black, none). If a Color is anything else, I want to know about it, because it's probably a bug.
I do not like this approach. IMHO, it concentrates on wrong details. It is well known that the number of bugs a programmer emits is proportional to the length of the program. Wrapping everything into class will add a complexity that translates into more bugs in the program. Always checking the invariants will also make the program slower. All this is the way of programming bloated code.
If I didn't use classes to wrap these simple types, I would use enums like Stockfish.
enums is a better approach, although I think C++ is too pernickety about them.
Richard