From the perspective of someone who generally writes far fewer comments than this, I find that this code has a number of comments which is well-justified due to the complexity of the subject, poor availability of external documentation describing the interface it implements, and the nature of the code. However, the code can be written in a manner which is still readable with fewer comments.
Here's a sample from the file:
// For sprites, color 0 is transparency so don't draw anything.
if color == 0 {
continue;
}
This comment is probably not necessary: it would be equally clear if it were written as something like the following:
if color == SPRITE_COLOR_TRANSPARENT {
continue;
}
A few lines up is another comment:
// Is this specific pixel not on the screen? We already check that x_pos is not off
// the left side earlier, so only need to check that it's not off the right side.
if x_pos + p >= 160 {
continue;
}
If the code were written with a constant, then it would only need to clarify why we don't check the left side here (the only non-obvious thing about the code), so:
// Note: bounds testing on the left side is handled earlier
if x_pos + p >= SCREEN_WIDTH {
continue;
}
One last example:
// We want to iterate through 160 pixels to draw one scanline.
for col in 0..160u8 {
I like the changes but I think the original has merit too. Along with the comments it is documenting the parameters/bounds that apply to this device where they are actually being used, making the code a kind of documentation for others who want to implement similar things. Having these defined in constants elsewhere would require some back and forth if this "document" is used as a reference.
Well, I said upfront that the level of comments here is fine. Just offering a perspective on how it could still be good code with less. In general I think it's fine to expect a reading of the code to be paired with a reading of the documentation/specification; in the absence of such a resource for the GameBoy the comments here are well-justified. I would definitely add more constants, though, and I would anticipate that a second implementation which references this would start by re-defining all of the constants in its own code.
Most people will argue that this is nitpicking, however, I've learned something new today. Thank you.
Quite a difference in code readability to the point where the code becomes self-evident. Few people are able to reach a more verbose level of documentation, let alone make sure the code remains understandeable in the future.
Here's a sample from the file:
This comment is probably not necessary: it would be equally clear if it were written as something like the following: A few lines up is another comment: If the code were written with a constant, then it would only need to clarify why we don't check the left side here (the only non-obvious thing about the code), so: One last example: No comment, similar level of readability: