willprice / vhdl-computer

A computer written ground up in VHDL
willprice.org/vhdl-computer
GNU General Public License v3.0
1 stars 0 forks source link

Clean up redundant comments #7

Closed willprice closed 10 years ago

willprice commented 10 years ago

Why repeat the semantics of the code in comment? Take for example:

--- Module Architecture
architecture arch_rtl

I know it's the architecture as it's got the keyword architecture, I'd rather get my information from the text that actually contains semantics, and not the stuff that means nothing to the computer. Likewise for the entity declaration, and the library imports, why repeat yourself?

ben-marshall commented 10 years ago

I left that in deliberatly. Especially when it comes to HDL, there is no such thing as a redundant comment since it can be harder to parse manually than other languages.

That comment should really contain a description of the components within the module and how they are connected up etc. Because this is such a small module it got left as is.

Further one can have different architectures for the same entity declaration and switch between them - so you have to document them there.

willprice commented 10 years ago

Why do we need the src to be parseable?

That comment should really contain a description of the components within the module and how they are connected up etc. Because this is such a small module it got left as is.

Which comment in particular are you referring to? If this is about the @details directive, then I was just fixing a spelling mistakes as it previously was @detais (should have done that in a separate commit really!).

Why would one want different architectures for the same entity? Where would you use the different architectures?

willprice commented 10 years ago

Also, if the comments are acting as markers for parsing, shouldn't they be a little less ambiguous: e.g. <<PARSER:ARCHITECTURE>>, that way people aren't going to get confused that it's just a redundant comment to the reader, but in fact a marker for a parser.

ben-marshall commented 10 years ago

Sorry, I havent explained this well. I meant the source should be easily parsable by a human being trying to get to grips with the code in a short time. Especially with port lists and so on.

the @details etc are markers for doxygen which is going to help us maintain all of the code documentation so we dont need to look at source files - we just have an API reference.

Different architectures for the same entity are used at different parts of the development cycle. Initially when you just want a working design you have a behavioural architecture that cheats lots using if statements, variables, for loops etc. Later when you know your design is viable, you write a synthasisable architecture using only signals to give you more control over the final RTL.

I am not sure how much we will use this approach. IF the modules are kept small in terms of lines of code then we can go straight to RTL.

Does that help?

willprice commented 10 years ago

I'm familiar with doxygen, and understand the use of those directives. I'm still not convinced about the comments above entity and architecture declarations, I'd personally just use whitespace for visual separation, that way you don't repeat what the code says in a comment.

Different architectures for the same entity are used at different parts of the development cycle. Initially when you just want a working design you have a behavioural architecture that cheats lots using if statements, variables, for loops etc. Later when you know your design is viable, you write a synthasisable architecture using only signals to give you more control over the final RTL.

I am not sure how much we will use this approach. IF the modules are kept small in terms of lines of > code then we can go straight to RTL.

That makes sense, thanks for clarifying.

willprice commented 10 years ago

When writing in any other PL, I never write a comment indicating that I'm going to declare a function named my_fabulous_function that returns int then write a function definition that says int my_fabulous_function, for starters it means that the comment will get out of sync when I perform automated refactorings, it also adds visual noise (IMO), and repeats what the code says, but in a non-typed way. Comments are a code smell that indicate you should probably refactor your code out to smaller functions encapsulating the sentiment of the comment in a function name.

A good discussion on the use of comments in code here: http://programmers.stackexchange.com/questions/1/comments-are-a-code-smell Comments are for why, not how, how should be expressed in function names.

What do you think?

ben-marshall commented 10 years ago

There is a balance to be struck that leans heavily toward the why for sure.

My opinion is better to have and not need than need and not have. For small modules lets leave them out and larger ones just have an explanation of what the module is for.

willprice commented 10 years ago

I agree putting a high level description of a module is good practise on large modules that require it, but I have a problem with redundant comments, I'm glad we can strike a happy medium. Thank you for compromising :-) On 15 Aug 2014 10:37, "Ben Marshall" notifications@github.com wrote:

There is a balance to be struck that leans heavily toward the why for sure.

My opinion is better to have and not need than need and not have. For small modules lets leave them out and larger ones just have an explanation of what the module is for.

— Reply to this email directly or view it on GitHub https://github.com/willprice/vhdl-computer/pull/7#issuecomment-52289766.

ben-marshall commented 10 years ago

No problem. Lest we resort to git blame

willprice commented 10 years ago

God forbid!