SAP / styleguides

This repository provides SAP style guides for coding and coding-related topics.
Other
1.66k stars 441 forks source link

[Comment Position] what is wrong with inline comments? #297

Open schibbi opened 1 year ago

schibbi commented 1 year ago

Relevant sections of the style guide https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#put-comments-before-the-statement-they-relate-to

Question What exactly is meant with "invasive" related to short comments for short lines of code like output = calculate_result( input ). " delegate pattern ? Or asking more generally: what is the problem with such comments?

a) I find such inline-comments very clear. b) I would consider them more compact and less invasive then adding a new line just for the comment above the statement c) I prefer inline comments over comments in separate lines, because I have seen many cases where separate-line-comments got disconnected from their related statement because of "not so diligent" code changes, so that a (potentially helpful) comment got completely confusing, because the statement being several lines of code "away" from the comment now...

Any insights or comments? I also did not find anything specific to comment positions in the web (but lot's of stuff related to the content of comments).

fabianlupa commented 1 year ago

I think they are mostly fine as well. Some aspects come to mind that might favor the separate line(s):

Jelena-P commented 1 year ago

Came here to comment about the comments. :)

Not sure where this suggestion came from, it's first time I'm hearing a recommendation to use " instead of *. It's an interesting point that " comment indents with the command but then (a) is this really important? (b) the example shown should not require a comment to begin with. It's the whole point of Clean ABAP to avoid unnecessary comments and use clear names for variables and methods.

Also, the comment/uncomment keyboard shortcuts in both SAP GUI and Eclipse use comments. I write comments only to explain something that is not clear from the code, so it takes a few lines. I write the comment, do nice formatting and then use keyboard shortcut to make it a comment. Which would add . ¯_(ツ)_/¯

My recommendation would be to use these as intended: full lines of comments with , inline comments with ". I rarely use inline comments these days, so suggesting to use " instead of (as it's currently written) is a very weird one.

Edit: @schibbi I also agree with your point that if the comment pertains to one line, then inline comment is actually a better approach for the reasons you stated.

fabianlupa commented 1 year ago

@Jelena-P So you write a multiline comment without any * or " and then press Ctrl+< after being done? I can't imagine that at all, the comment will be underlined red the whole way while writing it as it's a syntax error until you are done (in ADT)?! Also naturally on manual line break you will be at the wrong position and you'll have to correct afterwards with multi line selection (Ctrl+shift+a)? I also cannot think of another language where comments aren't automatically aligned with the code blocks around them so to me " is the expected behavior, to your point (a).

ConjuringCoffee commented 1 year ago

I've just broadly checked the Clean Code book by Robert C. Martin and I wasn't able to find any explicit mention of this rule. He seems to have a preference against inline comments though, seeing as the only example I was able to find was from a bad practice example.

The closest rule I was able to find was the one against comments behind closing brackets. I've seen that in ABAP as well, like this:

ENDIF. " IF sy-subrc = 0

The preference against inline comments is also in line with Martin's formatting rules which prefer shorter code lines.

As a side note, I like that it prevents versioning comments like this:

g_test = 1. "GHL080498

However, I do agree that there are cases in which inline comments make more sense. I think these cases are very rare though. I would be fine with removing the rule because the preference against them is still conveyed by other rules.

Jelena-P commented 1 year ago

@fabianlupa The key here is to use SE80 where nothing is highlighted in red. :) Also, sometimes I even write the comments some place else and copy-paste them. Obviously I try to do self-documenting code as much as possible but sometimes 2-3 lines worth of comments are really needed. I am aware I'm likely putting much more thought into the comment text than an average ABAPer. :)

Jelena-P commented 1 year ago

@ConjuringCoffee The "versioning comments" (which I hate with passion) are already addressed by another point.

After looking at the comments here, I would change the suggestion to point out the indentation feature of the inline comments but to provide no opinion otherwise. Both can be used whenever appropriate. There is nothing more annoying than 3 lines of comments preceded by an inline comment sign AND varying (!) number of spaces in front of it. But I'm not sure how to formulate a suggestion on this without sounding too obsessive.

ConjuringCoffee commented 1 year ago

There is nothing more annoying than 3 lines of comments preceded by an inline comment sign AND varying (!) number of spaces in front of it. But I'm not sure how to formulate a suggestion on this without sounding too obsessive.

@Jelena-P Could you please make an example for such a case?

Jelena-P commented 1 year ago

@ConjuringCoffee It would look something like this:

image

JozsefSzikszai commented 1 year ago

image

Why would it look like this? Pretty Printer aligns the apostrophes.

Jelena-P commented 1 year ago

@JozsefSzikszai IDK, some people still don't use PP for some reason. I was literally looking at a comment like this when I went to check what does Clean ABAP say about this and then stumbled upon this post.

With PP, here is also what happens:

image

*mean

fabianlupa commented 1 year ago

Someone must have entered those spaces after the " manually. I'd argue that person didn't try at all to align things correctly. * Wouldn't help that either.

JozsefSzikszai commented 1 year ago

@Jelena-P Any clients I have been working in the past 10+ years, using PP (and with exact setup of PP) was part of the development guidelines. Not using PP, also means failing the Code Review. If a developer uses random number of spaces in their comments (which for me is a sign of not paying attention to details), I assume there will be more issues in the code itself.