Get rid of unnecessary comments

[NikitaLejnev]

Idea

Comments are supposed to improve readability and make it easier to reason about the code. Currently there are a lot of comments that are redundant. There is no sense in commenting a function if its name clearly describes its purpose. There is no sense in commenting blocks of code in a large method, it is much better to extract them into their own functions.

Todo

• If you see a comment that doesn't add important information, please remove it.
• If you want to comment something, please consider another way of communicating what you're trying to achieve.

[Nokkvi]

I agree that many comments in the repo add little to no value, can I take this issue?

[William-McGonagle]

I agree that comments are supposed to improve readability, but we want the onboarding process for new contributors to be as quick as possible, and that is done through somewhat redundant comments. So, if you can find a way to make a redundant comment less redundant, do that, but don't remove comments that are already there.

https://mitcommlab.mit.edu/broad/commkit/coding-and-comment-style/

We are using the MIT guide for commenting, and on top of that, we have been using Google's idea that comments should break up code flow. What this means is in the example below, a comment should be used to break up the separate ideas in the code (just like a period or comma in a sentence), as it increases readability.

Bad:

// Create the Badge and Text
var resultTitle = "\033[1m";
if (result == true) resultTitle += "\033[1;37;42m PASSED ";
if (result == false) resultTitle += "\033[1;37;41m FAILED ";
resultTitle += "\033[22m";
resultTitle += "\033[0m";
return `${resultTitle} ${text}`;

Good:

// Set Title to Be Bold
var resultTitle = "\033[1m";

// Create the Badge Text
if (result == true) resultTitle += "\033[1;37;42m PASSED ";
if (result == false) resultTitle += "\033[1;37;41m FAILED ";

// Unbold Title
resultTitle += "\033[22m";
resultTitle += "\033[0m";

// Log to Console
return `${resultTitle} ${text}`;

[NikitaLejnev]

@William-McGonagle The point is to separate implementation details from higher level logic so you can get the general idea of what the method is doing without looking at the concrete implementation. Conversely, if you need to see how the function is implemented you look at it. I often extract the commented blocks into functions that are named after the comments. Two birds with one stone.

[William-McGonagle]

@NikitaLejnev I do have to say- I really like the new utility functions that you have added as they reduce the need for comments, but in the places like Sequelize references or high-level express stuff, we should keep them in because it makes the code easier for the new commiters.

[NikitaLejnev]

@William-McGonagle Fair enough. Thank you for being open to discussing ideas and suggestions. I like your idea and love the way you handle the project.

[Nokkvi]

@William-McGonagle, possibly I'm not understanding correctly, but my assumption for this issue was for removing comments that don't add value.

A example I found in the code:

function TokenizeString(input) {
  // Make it Lowercase
  const lowercase = input.toLowerCase();

  // Switch Around Similar Looking Symbols
  let partsRemoved = lowercase;
  partsRemoved = partsRemoved.replace(/\$/g, "s");
  partsRemoved = partsRemoved.replace(/@/g, "a");

  // Return the Final Result
  return partsRemoved;
}

Here the top and bottom comments are not adding value because the following line describes exactly the same thing.
But the middle comment adds value as it is not immediately obvious what that part of the code does.

Just my 2 cents, but I would like to keep working on this project, as I like the idea you've got here.``

[William-McGonagle]

@Nokkvi I would agree with your point that the first and last comment don't add value. But you are right that the middle comments should be kept because they somewhat help to explain the flow.

[Nokkvi]

#93