Commenting Code (in Progamming Q & A)

[MrSwiss]

Hallo to all,

There is really, really a wealth of information in certain threads. Plenty to learn from, but unfortunately seldom commented (the actual code samples/snippets).
This would IMHO dramatically lower the 'entry barrier' faced by beginner/intermediate programmers. I'm going to add some code from a recent thread as a sample:

Code: Select all

' ===== forum "Byte->string" / Q:VANYA A:fxm ===== 
' http://www.freebasic.net/forum/viewtopic.php?f=2&t=21326 '

Function i() As String                  ' func. i is to return a string
                                        ' () = NO parameters 'to' the func.
   Dim As Byte Ptr p = Allocate(10)     ' p = 0 to 9 (BASE = 0), allocates the
                                        ' memory used (10 bytes)
   For j As Integer = 0 To 9            ' j = index of p[] in for/next loop
      p[j] = j+83                       ' 83 = offset of "S" in ASCII-table
   Next                                 ' above -> Headache (figuring it out)

   Dim As String Ptr s =  Allocate(Sizeof(String))  ' see p above
   Cast(Any Ptr Ptr, s)[0] = p          ' define lbound p
   Cast(Integer Ptr, s)[1] = 10         ' define usefull string lenght (nr. of
                                        ' char)
   Cast(Integer Ptr, s)[2] = 10+1       ' define ubound p (terminator = chr(0))
   Function = *s                        ' dereferenced string ptr = string
                                        ' Functions 'return'
   DeAllocate s                         ' last Allocate -> DeAllocate first
   DeAllocate p                         ' first Allocate -> DeAllocate last
End Function                            ' necessary to prevent memory-leakage

? i                                     ' ? = print, in this context ?!?
                                        ' still has me guessing ...
sleep                                   ' wait for user action (keypess)
                                        ' to prevent immediate close of
                                        ' console 'output' ...

Thread reference given at top of code.
I'm not even certain if I got it correct in all instances (in my comments), so please correct me in case of error. I'm aware of the fact that many programmers dislike doing that but it would help a lot to understand the solution approach. At least when a specific problem has been solved. Comments wellcome ...

[Lachie Dazdarian]

I'm with you on this, man. Sadly not proficient enough in programming to cover a wider range of topics, beside some basic game dev tutorials I wrote.

[fxm]

Because I answered VANYA, I have not been paying attention that this topic was in the 'Beginners' forum!
This topic deserves to be moved to the 'General' forum.

I think my second version is better, only one memory allocation both for the string descriptor (12 bytes) and the byte/character data (10 bytes), and I consider this first version was just an help to better understand the second (with the line 2 improved to match a true descriptor):

Code: Select all

Function i() As String
   Dim As Byte Ptr p = CAllocate(Sizeof(String) + 10+1)
   For j As Integer = 0 To 9
      p[Sizeof(String) + j] = j+Asc("S")
   Next
   Cast(Any Ptr Ptr, p)[0] = p + Sizeof(String)
   Cast(Integer Ptr, p)[1] = 10
   Cast(Integer Ptr, p)[2] = 10+1
   Function = *Cast(String Ptr, p)
   DeAllocate p
End Function

Print i
Sleep

[Edit]
Replaced '83' by 'Asc("S")'.

[BasicCoder2]

As you get more experience the comments can be reduced as being told a hundreds of times what next i means would just be annoying. In the case of ? being used instead of PRINT it is really bad practice in my opinion and defeats the whole point of readability. The whole point of BASIC was to make it readable and simple to learn and use. I had never come across Function = *s before. As C-like features are added to BASIC it becomes a version of C with all the readability problems of C for a beginner.

I would have converted a list of bytes to a string with more BASIC like statement:

Code: Select all

'THIS FUNCTION TAKES A UBYTE ARRAY REPRESENTING ASCII CODES
'AND RETURNS THEM AS A STRING VARIABLE

function bytesToString(byteArray() as ubyte,lengthOfArray as integer) as string
    dim as string tempString                         'temporary string
    for i as integer = 0 to lengthOfArray-1            'set up loop paramaters
        tempString = tempString + chr(byteArray(i))  'add another ascii code to string
    next i                                           'add increment to i loop if less than lengthArray+1
    return tempString                                'return this value
end function

dim as ubyte bText(5)   'create a ubyte list
dim as string strText   'create a string

'fill ubyte list with asc values
bText(0) = asc("H")
bText(1) = asc("E")
bText(2) = asc("L")
bText(3) = asc("L")
bText(4) = asc("O")

'convert ubyte values in ubyte list of length 5 to a string
strText = bytesToString(bText(),5)
print strText

sleep

I used the statement len(byteArray()) in the function but it didn't work.

[MrSwiss]

@fxm

because I wrote to ... ...Beginners Forum

There is the inherent problem, you anwer to somebody you know ... and allready exclude the 'rest of the world', just a manner of speech ;-)
It was in the 'Programming' Forum ... my reference was to beginner/intermediate ... Programmers. I commented your first example because of it beeing easier to understand.

[Gonzo]

This article, one of many, gives some insight into why we comment code:
http://arstechnica.com/information-tech ... s-in-code/

general picture:
* don't comment obvious things
* code should be written as self explanatory, meaning you can read the code
* dim as integer a, b, c, x, y is not code, its trash
* if the code is complicated or has unusual side-effects, document it
* if the code is based on theories, theorems or documentation -
write title followed by hyperlink, no need to write something others can explain much better than you, more correctly

and so on :P

so, BasicCoder example code could be better documented:

Code: Select all

dim as ubyte bText(5)   'create a ubyte list
dim as string strText   'create a string
-->
dim as ubyte bText(5)   'list of XXXXX for YYYYY
dim as string strText   'XXXXX text used in YYYYY

Code in first post, don't comment on every line, just write some comments about
what the function does and why it exists. You could even provide where its being used from...
Just looking at it, I can't say it's anything other than a mystery function that handles magic and performs actions

[anonymous1337]

Code: Select all

'THIS FUNCTION TAKES A UBYTE ARRAY REPRESENTING ASCII CODES
'AND RETURNS THEM AS A STRING VARIABLE
function bytesToString(byteArray() as ubyte,lengthOfArray as integer) as string
    dim as string tempString                         'temporary string
    for i as integer = 0 to lengthOfArray-1            'set up loop paramaters
        tempString = tempString + chr(byteArray(i))  'add another ascii code to string
    next i                                           'add increment to i loop if less than lengthArray+1
    return tempString                                'return this value
end function

dim as ubyte bText(5)   'create a ubyte list
dim as string strText   'create a string

'fill ubyte list with asc values
bText(0) = asc("H")
bText(1) = asc("E")
bText(2) = asc("L")
bText(3) = asc("L")
bText(4) = asc("O")

'convert ubyte values in ubyte list of length 5 to a string
strText = bytesToString(bText(),5)
print strText

sleep

I used the statement len(byteArray()) in the function but it didn't work.

My opinion on this: If your comments say the exact same thing as your code - literally, not functionally, the exact same thing - they aren't needed.

Code: Select all

'THIS FUNCTION TAKES A UBYTE ARRAY REPRESENTING ASCII CODES
'AND RETURNS THEM AS A STRING VARIABLE

Yes, that's what the code says.

Code: Select all

'temporary string

... right. If we didn't want to have to explain dim as string tempString to people, maybe we should name variables stuff like temporaryString instead of acronyms/shorthands they're bound to misunderstand.

Code: Select all

'set up loop paramaters

This doesn't actually say anything. Yes, that's what's being done but if someone understands the concept of a loop or parameters at all, I guarantee you that line of code is not at all confusing.

Code: Select all

'add another ascii code to string

This comment seems incorrect, which is a shame because it's the only line of code in the entire sample someone is unlikely to understand. For the purpose of teaching people, this should also be broken down into two lines of code.

Code: Select all

var byteAsCharacter = chr(byteArray(i)) ' Convert the byte at the current index to a single-character string
tempString += byteAsCharacter  ' Append the character to the temporary string

(I also think "temporary string" is a misleading name. It's temporary in memory but it's really a "return value" or "result string". Something along those lines.

Code: Select all

'add increment to i loop if less than lengthArray+1

Confusing and distracting, IMO.

Code: Select all

'return this value

Unnecessarily redundant.

Code: Select all

'create a ubyte list

So is this.

Code: Select all

'create a string

And this.

Code: Select all

'fill ubyte list with asc values

Never mind. This line needed commenting, too. What in the world are ASC values? Anyone who knows that should understand the rest of this code.

Code: Select all

'convert ubyte values in ubyte list of length 5 to a string

This one, also.

I'm against Hungarian notation, personally. textString, byteList, etc. are so much better, IMO.

[dafhi]

Hi Gonzo, nice to see you back.

* dim as integer a, b, c, x, y is not code, its trash

this would be confusing to some. I think what you're getting at is it's best to go with

Code: Select all

Dim as integer count_apples = 5
For x_source as integer = 0 to 99
  ..

Definitely I resonate with the topic. dodicat, StoneMonkey, AGS, angros, pine, anonymous1337 and some others write readable code which is great to learn from

[fxm]

IMHO, the comment must remain at a functionnal level and not be just the description of keywords used (documentation is for that).

Here's how I could comment on my example if I had to do it:

Code: Select all

Function i() As String
   Dim As Byte Ptr p = CAllocate(Sizeof(String) + 10+1) ' memory allocation for:
                                                        '   - string descriptor (12 bytes) +
                                                        '   - useful byte/character data (10 bytes) +
                                                        '   - 1 byte for string compatibility
                                                        '     with null terminal character
   For j As Integer = 0 To 9
      p[Sizeof(String) + j] = j+Asc("S")                ' byte/character data initialisation
   Next
                                                        ' descriptor initialisation:
   Cast(Any Ptr Ptr, p)[0] = p + Sizeof(String)         '   - pointer to string character data
   Cast(Integer Ptr, p)[1] = 10                         '   - useful string character data size
   Cast(Integer Ptr, p)[2] = 10+1                       '   - allocated memory size
   
   Function = *Cast(String Ptr, p)                      ' returning string by value
   DeAllocate p                                         ' memory deallocation after string copy
End Function

Print i

[Edit]
Replaced '83' by 'Asc("S")'.

[MrSwiss]

[quote="fxm"]IMHO, the comment must remain at a functionnal level and not be just the description of keywords used (documentation is for that).

Here's how I could comment on my example if I had to do it:

Code: Select all

Function i() As String
   Dim As Byte Ptr p = CAllocate(Sizeof(String) + 10+1) ' memory allocation for:
                                                        '   - string descriptor (12 bytes) +
                                                        '   - useful byte/character data (10 bytes) +
                                                        '   - 1 byte for string compatibility
                                                        '     with null terminal character
   For j As Integer = 0 To 9
      p[Sizeof(String) + j] = j+83                      ' byte/character data initialisation
   Next

I'd go with that with one exeption, which seems to break your 'rule'. The exeption being the IMHO missing explanation of '83' (it obviously being an offset) but to what?!? I've only figured that one out after compiling/running the code. In such a case a little bit more (offset in ASCII-table) explains a hell of a lot, this had me running in a loop (as explained above) actually distracting from 'the real issue'. Hope that this explains my claim for a little bit 'more' explanation. Also, because the refered Documentation is nonexisting (in this case).

[fxm]

p[Sizeof(String) + j] = j+Asc("S")

[BasicCoder2]

@anonymous1337

I take on board your comments on my commenting in the above code.

To see how I usually comment for my own use search forums for BasicCoder2.

One thing that is often lacking is an explanation of the algorithms so you don't have to reverse engineer the example code. Such explanations can be used to translate down to any computer programming language.

[anonymous1337]

Algorithm implementations depend on what language features are available and aren't generically portable.

Algorithm explanations require domain-specific knowledge.

Typically, you implement an algorithm, telling people what algorithm you implemented with a moderate abstract describing the method.

The nature of optimized algorithms means that most of the implementation will make no sense to someone who doesn't understand the problem and implementation abstract. Someone who understands what's going on should understand the code just fine, 60% of the time.

I don't find code comments as a solution to this problem. They tend to be as confusing as the code itself. People who have domain-specific expertise are largely awful at dumbing things down. Code comments aren't really fit for explaining those things which are better modeled graphically.

So my thoughts on this are mixed. It would be nice if helpful comments could be added. The question is whether or not helpful comments exist. Probably an NP-hard problem... :P

[Gonzo]

well, you implemented that algorithm somehow
just put a link to the source in a comment and call it a day

if you made it up yourself, chances are you'd want to document it, if only for your own sake
so, just do it while you remember what you did, or at least why

for all the hyper-optimized algorithms ive written to lessen the impact of bottlenecks,
i probably haven't documented any of them.. the only good thing about them is code readability
i once wrote a multi-threaded octtree-like implementation which performed just as well as my previous hyper-optimized recursive special case implementation, so, shortly after i just reverted all the changes.. i didn't gain enough cycles to care enough about totally unreadable code

for all the good commenting does, it has drawbacks, such as that the comments can lie :)
more than anything writing clear concise code that is properly split up into digestible parts is key

[MrSwiss]

p[Sizeof(String) + j] = j+Asc("S")

@fxm, you made it ... readable code (no comment needed), thank you.

... this are mixed. It would be nice if helpful comments could be added. The question is whether or not helpful comments exist. Probably an NP-hard problem... :P

@Prichard, I suppose it is similar to the problem one faces when holding a presentation. To be successfull it has to be adapted to the audience present. IMHO it is better practice to start a bit 'to low', adapting to higher as one goes (if the audience turns out to have more knowledge as expected), because if one starts 'to high' the audience is allready 'lost' at this point.

... one thing that is often lacking is an explanation of the algorithms so you don't have to reverse engineer the example code.

@BasicCoder2, you got your point, I tend to agree.

well, you implemented that algorithm somehow
just put a link to the source in a comment and call it a day
if you made it up yourself, chances are you'd want to document it, if only for your own sake
so, just do it while you remember what you did, or at least why

@Gonzo, seems to me sound programming advice, like the one I read elsewhere:
"If you can't read your own code after 6 months, it is time to comment it."

Thank you all for your contributions.