Winboard Forum

Note that I read in the past that comments inside the code may cause problems to understand the code so I suggest comments not inside the code but as text files.

For example
there is square.h in fruit2.1 so comments to square.h can be saved in squareh.txt and the same for other files and people who read some file and do not understand can go to the txt file that may help them.

I may release comments to part of fruit files but only if I am not the only person who does it and other people also spend time in commenting fruit files so their work can help me to understand parts of fruit and not only I help other to understand parts of fruit.

Uri

Probably, only Fabian can do a really excellent job of that.

A wrong comment will be far more damaging than several correct ones can make things better.

Probably, only Fabian can do a really excellent job of that.

A wrong comment will be far more damaging than several correct ones can make things better.

I do not agree that only fabien can do a good job there.

I think that avoiding wrong comments is no problem and if I am not sure that I understand something I will write that I am not sure that I understand it in commenting fruit.

In case that there is going to be some project of commenting fruit then I expect myself not to do errors in commenting the files that I am responsible for them(I am less sure if I am going to understand everything and that my comments will not include notes like "not understanding lines 45-47" or "not sure if lines 45-47 are for...").

Uri

Why comment code, when you can write papers on algorithms/techniques/tweaking in Fruit?

Hi Uri and all others,

one problem of putting comments into separate files might be the maintenance problem. Separate documentation has to be changed synchronously to changes in the source code; do you think this would really work? Today you only think of Fruit 2.1 but what about future versions? My experience over years is that commenting directly within the source code is the only acceptable approach while detailled separate documentation (I don't call it "comments" due to its different style of being an external document) is outdated most of its lifetime.

That's why I fully agree with Dann in that this can only be done by the author himself, provided he wants to do it. Commenting done by anyone else is a quite dangerous task, as Dann already stated, because it may produce comments looking like an explanation of what the author had in mind when writing a piece of code - which is impossible to tell by someone else than himself.

Btw I don't think that Fruit 2.1 source code really needs much more commenting; actually it depends on the level of understanding you want to achieve: understanding the higher-level, functional aspects of some code or understanding lower-level, internal implementation details. For the latter, there may be a need to add a few comments but I am not sure if this is important.

I want to add a personal note on code commenting.

My experience in software development taught me that commenting is essential for "surviving". If you work on different tasks over a certain period of time, or even if different people work on the same task (at the same time or sequentially), you absolutely need excellent code commenting, otherwise you will be lost at some point.

However, most computer chess projects are single person projects, and for many authors it is their main project for a longer period, so things may be different here. Personally I'm a fan of commenting, and I try to apply software quality criteria to my own, small chess project in the same way as I have to do it for my customer at work. But of course this is not the only possible way of doing it.

Sven

Sven Sch?le wrote:Hi Uri and all others,

one problem of putting comments into separate files might be the maintenance problem. Separate documentation has to be changed synchronously to changes in the source code; do you think this would really work? Today you only think of Fruit 2.1 but what about future versions?

I think that commenting Fruit2.1 is better than nothing and it is good to understand Fruit2.1 without understanding future versions.

I also think that a lot of files in Fruit2.1 will be unchanged in future version so the comments for them will be relevant also for future versions.

Sven Sch?le wrote:
My experience over years is that commenting directly within the source code is the only acceptable approach while detailled separate documentation (I don't call it "comments" due to its different style of being an external document) is outdated most of its lifetime.

I think that seperate documentation is specifically for Fruit2.1
Assuming that most of the data structure of Fruit will be the same I think that understanding Fruit2.1 can help later to understand also future fruit versions.

Sven Sch?le wrote:That's why I fully agree with Dann in that this can only be done by the author himself, provided he wants to do it. Commenting done by anyone else is a quite dangerous task, as Dann already stated, because it may produce comments looking like an explanation of what the author had in mind when writing a piece of code - which is impossible to tell by someone else than himself.

I disagree that it is impossible to tell.
I can give an example from eval.cpp of fruit about some commenting of king safety evaluation to explain what I mean.

It starts with

if (UseKingAttack)

possible comment is that UseKingAttack=true by default and it was not obvious for me as a reader and I needed to search for UseKingAttack to find it(It was not a long time to do it but I printed all fruit source and if I want to try to understand fruit source by reading when I am not near the computer finding the value of the constant UseKingAttack can take sifgnificant time and even near the computer I do not like to see often

edit->finds in files

Later I see
for (colour = 0; colour < ColourNb; colour++)

I could guess that ColourNb=2 but again I do not want to have misunderstanding so again I search for the string

It appears many times and I need some seconds to see
const int ColourNb = 2 in colour.h

Later I have
if ((mat_info->cflags[colour] & MatKingFlag) != 0)

I understand that it means that the opponent has at least queen and at least 2 pieces but it took me a long time to understand it.

first searching for MatKingFlag gave
const int MatKingFlag = 1 << 3

Later I needed to understand what is
mat_info->cflags[colour]

I figured out that I need to look at material.cpp and find that it has

if (bq >= 1 && bq+br+bb+bn >= 2) cflags[White] |= MatKingFlag;
if (wq >= 1 && wq+wr+wb+wn >= 2) cflags[Black] |= MatKingFlag;

even at this stage it is only a guess that bq>=1 means that black has at least one queen and I need to look at the code to be sure that my guess is right.

only seeing lines like
bq = board->number[BlackQueen12];

remove doubt(I assume that the author did not choose clearly illogical names).

Sven Sch?le wrote:Btw I don't think that Fruit 2.1 source code really needs much more commenting; actually it depends on the level of understanding you want to achieve: understanding the higher-level, functional aspects of some code or understanding lower-level, internal implementation details. For the latter, there may be a need to add a few comments but I am not sure if this is important.

I want to understand both and I do not feel sure of understanding the higher-level without understanding the lower level.
I showed an example why I think that it does need commenting.

Sven Sch?le wrote:
I want to add a personal note on code commenting.
My experience in software development taught me that commenting is essential for "surviving". If you work on different tasks over a certain period of time, or even if different people work on the same task (at the same time or sequentially), you absolutely need excellent code commenting, otherwise you will be lost at some point.

However, most computer chess projects are single person projects, and for many authors it is their main project for a longer period, so things may be different here.
Sven

I think that in the case of Fruit the target of the code is that other people will also work on the code and try to improve it and I think that understanding the code can be productive to improve it.

It was not meant to be only a project of Fabien(otherwise there is no reason for him to release the source code)

I do not plan to try to improve fruit but it does not mean that I do not want to understand it and comments should help people to spend less time in understanding the code(I think that situation when you need a lot of time to understand some code is not encouraging people to try to understand it).

Hi Uri,

just some notes on your code examples. Perhaps this may be interesting for Fabien, too.

Code: Select all

if (UseKingAttack)

I think the name of this constant has been chosen good enough here to understand its meaning immediately, although a name like "isKingAttackUsageConfigured" might be even more precise (some people don't like very long identifiers, but these have the advantage of not requiring any additional comment in most cases).

Code: Select all

for (colour = 0; colour < ColourNb; colour++)

In general, it should not be necessary to know the exact values of symbolic constants like "ColourNb", otherwise you would not really need the constant, you could write "colour < 2" instead. The reader can imagine that the loop's body is run once for each available colour. Again, even a bit more readable in my opinion (and not requiring any comment) might be code like this:

Code: Select all

for (colour = White; colour <= Black; colour++)

assuming that Black == White+1.

Code: Select all

if ((mat_info->cflags[colour] & MatKingFlag) != 0)

This is an example where I admit that comments are necessary. An inline function taking mat_info and colour as arguments and returning bool, with an appropriate name, would avoid the need for comments, though.

Code: Select all

if (bq >= 1 && bq+br+bb+bn >= 2) cflags[White] |= MatKingFlag;
if (wq >= 1 && wq+wr+wb+wn >= 2) cflags[Black] |= MatKingFlag;

This one's a bit difficult to judge. Putting such code into separate small (inline) functions might be overkill, so there should be a comment on these two lines.

In general, I think that choosing identifiers carefully and structuring the program in a straightforward and simple manner can already avoid many source code comments.

Regarding Fruit, I think you have chosen good examples for the commenting topic but again I would like to say that the Fruit source code is not difficult to understand in general, at least I did not encounter serious problems with it.

Sven

Hi Sven,

I do not think that it is easy to understand fruit's code in general.
I am interested first in understanding the king safety code of Fruit and I do not fully understand it at this moment.
basically there are 3 variables that are relevant and all of them are true by default
1)UseKingAttack
2)UseShelter
3)UseStorm

I find the code hard to understand
I guess that I may understand it if I invest enough time but it is not obvious for the reader what fruit does.

I believe that I understood the part of UseKingAttack but even in this case I am not sure about it.

I find the part of UseShelter and UseStorm extremely hard to understand

I see that the function shelter_square is used and I even do not understand what this function calculates.

shelter_square use the functions storm_file and shelter_file so you need first to understand what these functions do

storm_file use the definition BIT_LAST and it has
#define BIT_LAST(b) (BitLast[b])

I am not 100% sure what is the content of BitLast and without
reading pawn.cpp I cannot know what BitLast means

If I understand correctly pawn.cpp bitlast gives the last rank.
Note also that understanding the meaning of bitlast is not enough and I need to understand the exact structure of fruit in order to understand things like
board->pawn_file[COLOUR_OPP(colour)][file]

You can see by describing how I try to understand it that it is not an easy job thanks to the fact that fruit is not commented and it seems that eval.cpp is hard to understand and it is better if you first fully understand
files of fruit like pawn.cpp

If Fruit is well commented then humans may understand what the evaluation does thanks to comments without needing to understand the exact structure of fruit.

Note that I think that it is not relevant only for chess programmers and I think that chess players also may be interested to know how to calculate fruit's evaluation and maybe it can help them to be better players.

Uri

Sven Sch?le wrote:

Code: Select all

if ((mat_info->cflags[colour] & MatKingFlag) != 0)

This is an example where I admit that comments are necessary.

I think it is quite easy to guess what this code does, but as always, YMMV.

An inline function taking mat_info and colour as arguments and returning bool, with an appropriate name, would avoid the need for comments, though.

Unfortunatly inline function are not very useful in C, because the compiler is free to ignore them. GCC never inlines calls to a function defined in a different source file, for instance. For this reason, I normally use #define macros instead, as much as I hate them.

I agree that Fruit's source code is very easy and pleasant to read and understand without much comments. This is a characteristic of well-written code in general, in my opinion. I prefer reading and working with code which is compact, has good abstractions, well chosen identifiers, and where the comments are kept to a minimum. Verbose and overcommented code (e.g. Crafty) is tedious to read. Like so much else, this is of course largely a matter of taste.

Tord

Tord Romstad wrote:

Sven Sch?le wrote:

Code: Select all

if ((mat_info->cflags[colour] & MatKingFlag) != 0)

This is an example where I admit that comments are necessary.

I think it is quite easy to guess what this code does, but as always, YMMV.

Tord

I see no way to understand that
((mat_info->cflags[colour] & MatKingFlag) != 0) if and only if the opponent has at least 2 pieces and at least a queen.

It is not obvious that king safety is irrelevant if the opponent has no queen and I think that it is wrong.

I also do not like to guess without being sure because even if I am right in 90% of the cases then the 10% of the cases that I am wrong may lead to misunderstanding of the code.

Uri

Uri Blass wrote:I see no way to understand that
((mat_info->cflags[colour] & MatKingFlag) != 0) if and only if the opponent has at least 2 pieces and at least a queen.

No, of course you cannot see that it means exactly that the opponent has 2 pieces, at least one of which is a queen. But given the context, it is quite clear that the condition is supposed to mean something like "the other side has enough material that we should worry about king safety". You can't see exactly what Fruit considers "enough material" without looking elsewhere in the code, but this information isn't extremely interesting. I prefer to have such minute details hidden in a separate source file, instead of having it staring at me in the form of a comment every time I look at the code.

It is not obvious that king safety is irrelevant if the opponent has no queen and I think that it is wrong.

This is very possible, and it is actually one of the reasons why I don't think it is very important to be able to see immediately exactly how much material Fruit thinks a player needs to produce a dangerous attack. As you point out, the exact condition chosen is likely to be sub-optimal anyway, and has little value to the reader.

I also do not like to guess without being sure because even if I am right in 90% of the cases then the 10% of the cases that I am wrong may lead to misunderstanding of the code.

Then you can just zoom in to the low-level details in those few cases when it is necessary to understand everything 100%. With well-written code like Fruit, it isn't such a difficult task, but even so I am not sure it is worth the effort. I think you can learn a lot more by trying to emulate Fabien's coding style than by spending lots of time studying the most low-level parts of Fruit's source code.

Tord

Tord Romstad wrote:

Uri Blass wrote: It is not obvious that king safety is irrelevant if the opponent has no queen and I think that it is wrong.

This is very possible, and it is actually one of the reasons why I don't think it is very important to be able to see immediately exactly how much material Fruit thinks a player needs to produce a dangerous attack. As you point out, the exact condition chosen is likely to be sub-optimal anyway, and has little value to the reader.

Tord

I think that inspite of the fact that the condition is suboptimal it has value for the readers.

Fruit is strong so Fruit is probably at least almost right in its evaluation.
If I want to give bonus in cases that fruit does not give bonus then it is probably better if I am careful to give only small bonuses in these cases
and in order to know cases when it may be better to be more careful to give small bonus it is better to understand fruit's code.

Uri

Tord Romstad wrote:

Sven Sch?le wrote:An inline function taking mat_info and colour as arguments and returning bool, with an appropriate name, would avoid the need for comments, though.

Unfortunatly inline function are not very useful in C, because the compiler is free to ignore them. GCC never inlines calls to a function defined in a different source file, for instance. For this reason, I normally use #define macros instead, as much as I hate them.

Hi Tord,

if you write

Code: Select all

inline bool f(SomeType t)
{
    return someExpr(t);
}


and you want f() always to be inlined then put this function body (with "inline") into the header file where the function prototype of f() resides, and which you #include where you call f(). In this case, you don't even need the prototype anymore in most cases (except if you have another inline function in the same header file which calls f() and has its body defined before the body of f()). Don't put an inline function body into a cpp file if you want to call it also from outside. A header file is the appropriate place for it.

I know, of course, that some compilers do not inline some functions even if you use the "inline" keyword, but often the compiler has good reasons not to do it.

Sven

Sven Sch?le wrote:{snip}
I know, of course, that some compilers do not inline some functions even if you use the "inline" keyword, but often the compiler has good reasons not to do it.

Sven

Not always.
Sometimes, it will refuse just because it can't see the implementation (GCC does this).

So you have to resort to the cheesy #include source files trick:

#include "prjhdr.h"
#include "foo.c" /* First source file in project */
#include "bar.c"
#include "boo.c"
...
#include "end.c" /* Last source file in project */

However, using macros for functions has some very, very ugly side effects including undefined behavior if you look sideways at it.

Hi Dann,

Dann Corbit wrote:So you have to resort to the cheesy #include source files trick:

as I wrote above this is not the only way. Putting inline implementations into .h files instead, not .cpp files, will work.

Sven

Dann Corbit wrote:

Sven Sch?le wrote:{snip}
I know, of course, that some compilers do not inline some functions even if you use the "inline" keyword, but often the compiler has good reasons not to do it.

Sven

Not always.
Sometimes, it will refuse just because it can't see the implementation (GCC does this).

So you have to resort to the cheesy #include source files trick:

#include "prjhdr.h"
#include "foo.c" /* First source file in project */
#include "bar.c"
#include "boo.c"
...
#include "end.c" /* Last source file in project */

This works, but has the unfortunate consequence that you cannot define 'static' functions with the same name in different source files. I like using the 'static' keyword to define functions without polluting the global name space.

However, using macros for functions has some very, very ugly side effects including undefined behavior if you look sideways at it.

It has. As I said, I really hate using #define macros in C.

Tord

Sven Sch?le wrote:Putting inline implementations into .h files instead, not .cpp files, will work.

I suspect that GCC would complain that the same function in multiple source files in that case (I haven't tried, though). Perhaps defining the functions as 'static inline' and placing them in .h files would work.

Tord

Tord Romstad wrote:

Sven Sch?le wrote:Putting inline implementations into .h files instead, not .cpp files, will work.

I suspect that GCC would complain that the same function in multiple source files in that case (I haven't tried, though). Perhaps defining the functions as 'static inline' and placing them in .h files would work.

Tord

Hi Tord,

In fact inline function must be implemented in the .h file. The compiler will not complain. Since the function is inlined it's definition must be know while compiling a call to it, this is why you must put the definition of the function in the header file. Otherwise inline is ignored

mathmoi wrote:

Tord Romstad wrote:

Sven Sch?le wrote:Putting inline implementations into .h files instead, not .cpp files, will work.

I suspect that GCC would complain that the same function in multiple source files in that case (I haven't tried, though). Perhaps defining the functions as 'static inline' and placing them in .h files would work.

Tord

Hi Tord,

In fact inline function must be implemented in the .h file. The compiler will not complain. Since the function is inlined it's definition must be know while compiling a call to it, this is why you must put the definition of the function in the header file. Otherwise inline is ignored

Strangely, most other compilers get it right (MS, Intel, Compaq, etc.)

There is no reason that the compiler cannot rearrange things to its liking.

The GCC design is just lazy.