lidgren / lidgren-network-gen3

Lidgren Network Library
https://groups.google.com/forum/#!forum/lidgren-network-gen3
MIT License
1.19k stars 331 forks source link

Changed NetBuffer bool write to return bool value #24

Closed petesimard closed 9 years ago

petesimard commented 9 years ago

Allows writing easier branching server code by eliminating redundant checks

Instead of writing:

buffer.write(player != null);
if(player != null)
{
    buffer.write(player.name);
}

You can write:

if(buffer.write(player != null))
{
    buffer.write(player.name);
}
lidgren commented 9 years ago

No other Write* method returns a value and one might think the value returned is an indication of success or not; the syntactical advantages of this change does not outweigh those negatives imho.

petesimard commented 9 years ago

Are you sure about the syntactical advantage being so small?

The pattern I used in the example is very common in server code and not having a return value for bool means duplicating code and violating DRY. If you ever need to change the check and you forget to change it in both places, you've introduced a bug into your codebase. The comment makes it clear what the return value is and I don't think it would cause much if any confusion.

musmuris commented 9 years ago

I saw this and didn't say anything as I have no authority here - but I agree a write method returning a bool would commonly be a "yes I wrote this successfully" or "sorry i failed to write it" and not just returning the value passed in, just to save the line of code. The first one is obvious what is going on, the second one I have to go and read the comments to work it out.

You could maybe have a (and this name sucks) WritePossibleNullString and the corresponding ReadPossibleNullString the other side to hide away the extra line.