Reading a line of text from a socket

I tend to write code, with very few comments, and few line breaks. I'm currently just a student at a University. I've had professors tell me one way or another to comment, but I have a hard time with more ambiguous tasks.

I have a misc. snippet of code, written by me in C++. I'm not looking for revisions on it or anything. Just how, in your professional field, would you go about commenting it?

CString Socket::recvln()
{
    int size, count;
    char* result;

    OSFactory::lockMutex(recvLock);
    size = recvBuffer.size();
    result = new char[size+1];
    for (count = 0; count = size) // Should never >= size if '\n' found
        count = 0;
    result[count] = '\0';
    recvBuffer.erase(recvBuffer.begin(), recvBuffer.begin()+count);
    OSFactory::unlockMutex(recvLock);

    std::string _result = result;
    delete[] result;
    return _result;
}

Again, this is just a misc. method I have. I'm just trying to understand how other people would go about commenting code. There are tons of tutorials out there, but many even have you comment getter/setter methods, and to me that's really really pointless. Many times it's very clear that setName(std::string), for example, sets the Name!

So, I'm just trying to have a better grasp on real life commenting, versus text-book answers. For some reason this is really hard for me, and I'm sure it should benefit other people as well.

I don't see anything in this code that needs to be commented. To echo what other people said, comments should clarify code and they should never say what the code already says.

I would recommend commenting code under these conditions:

• You can't tell what the code is doing by reading it

• You encountered a wonky bug that took you a long time to figure out, and the fix does not look clear or uses a programming construct or pattern that isn't normally used.

Really, the biggest problem I see is one of the biggest problems in Software Engineering. As Martin Fowler once said:


... there are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.

The names of your variables and methods could use a little polishing.

For instance, does Socket::recvln() mean:

• Receive Line

• Receive Lane

• Recover Line...

Even if names become a little long, make them obvious. Don't abbreviate. Some people say that abbreviating well known terms within your industry are OK. I'm from the camp that disagrees. Never assume that the next programmer is an expert in your industry.

When it comes to "reducing lines" I have mixed feelings about omitting optional curly brackets. It looks nicer, but becomes extra typing when you have to add one line of code. I go back and forth on that and have come to a compromise with myself.

-
If omitting the curly brackets makes the code a two-liner, that's OK

if (count >= size)
    count = 0;

-
If I would be omitting curly brackets for nested structures, I'll put the curly brackets in even if I don't need to

for (count = 0; count < size; count++)
{
    if (recvBuffer.at(count) == '\n')
    {
        break;
    }
    else
    {
        result[count] = recvBuffer.at(count);
    }
}

-
But sometimes within a nested structure, I'll omit the optional brackets if it is obvious:

for (count = 0; count < size; count++)
{
    if (recvBuffer.at(count) == '\n')
        break;
    else
        result[count] = recvBuffer.at(count);
}

-
If I have a structure with one-liners and multi liners, I never omit the curly brackets:

if (count > size)
{
    count = 0;
}
else
{
    foo = count + 10;
    bar = foo * -1;
}

Lastly, I recommend putting spaces around operators to make the code easier to read:

recvBuffer.erase(recvBuffer.begin(), recvBuffer.begin() + count);

(notice the spaces around the "+" sign)

StackExchange Code Review Q#69419, answer score: 11