[INFO] Good Coding Practices

[chynu]

General Tips on How To Write Healthy/Good Code

Things that come up after reviewing lots of code that you ignore and then feel bad about because it will bite you back later

but in all seriousness readable code makes productivity go up. also it really helps in the long run when things don't pile up to become a horrible mess. like on its own these small inconsistencies are fine nbd totally ok but when we have a growing codebase that will eventually be seen by people outside of our team, this stuff gets p important.

also i don't want to spend most of reviewing commenting on nitpicky things like these, so i'll just spew it all out here :o. this is not targeted at anyone it's just general rules we might want to follow to have the coding deities on our side

Unfinished code

If you're working on some task Y and you're making a pull request for some task X and you accidentally put the two in the same branch, please take the extra effort to either transfer your work on Y to another branch (or just have the foresight to do this from the beginning) or make sure your work on Y is at a reasonable point where the code is not broken. Maybe it has implemented the beginning of its feature and it is safe to go on develop. Complete or non-dysfunctional code on develop only, PLEASE!

ALSO don't leave print statements in your PRs. UNLESS they are part of informing the user or developer of what's going on. In that case make it as descriptive as possible! Instead of print("forward") consider print("Bot is moving forward!") or something friendly like that, lol

TODOs and old code

Alrighty, we're still refactoring so there's a lot of old code being thrown around here and there. But if you're making a PR please refrain from including huge chunks of commented out code from the old codebase. You can refer to it later by bookmarking it somewhere else, and it detracts from code readability and we might not even use it in the end... so why put it on develop?

If you're planning on implementing some function and you want that placeholder, please use TODOs! Preferred method is by MAKING AN ISSUE ON GITHUB and then marking the TODO as follows:

// TODO (#10): Re-implement automatically updating bot locations.

Notice how there's a #10 in parentheses. This is the issue number, and can be VERY USEFUL when referring back to a thread of comments/info about that task specifically. Also, it's super easy for people to claim issues and look through the codebase for any markers on where to begin. If we're open source we better act like it...!

Commenting out code

DO NOT leave commented out code in your pull requests to develop unless you explicitly state in a comment/TODO right before it that you are planning on removing that comment or the line itself in a quick follow up PR. At least give us the false sense of security that the unexecutable/useless line of code will become of use or will be discarded in the near future (even if it might actually take like two months for you to remove it or do something about it).

If it's temporary, then say it is!

<!-- TODO (#84): Uncomment after JavaScript is written out. Currently causes
  a bug with the below line uncommented. -->
<!-- <div id="name01" class="profile">Bob Smith</div> -->
<div id="summary_box">
  <p>Lorem ipsum dolor sit amet...</p>
</div>

Or better yet, if you think it's not going to be used, then just remove it. On that note, don't comment out logs (print(), console.log(), whatever it is) if you're not gonna use it. Please remove those code weeds thnk u

Documentation

How 2 doc

Look at this page for more detailed information on how to document in JSdoc / Pythondoc(?) properly. Let's try to stick to this standard. This means writing documentation for every new function and making sure parameters are properly named and well documented. I'm talking about params like b instead of bot (yes, I've done this too, but let's all try together).

Here's an example for JS.

/**
 * This function does some bomb-a** stuff. It's the best javascript
 * function in the world.
 * @param {string} codeName Name of the coolest code(?) in the world.
 * @returns {Array<string>} List of cool names, or null if there are none.
 */
writeGoodCode(codeName) {
  // lol some cool code goes here
  return null;
}

Yeah honestly I still don't know how the documentation works for Python but let's just try to follow that link above, thx (it includes info on Python as well)

def function_name(param):
  """
  Does some cool stuff. Here's a descriptive description lol
  
  Args:
    param (obj:`str`): Some parameter that does something.
  Returns:
    obj:`str`: Just the string "swag" and nothing else.
  """
  # TODO: Implement this cool function.
  return "swag"

how 2 writing

USE ACTION VERBS PLS MAKE THE SENTENCES SHORT N TO THE POINT and start w verbs. reference other functions if it will always depend on those other functions.

e.g.
Good function description:

/**
 * Parses through given message, extracts key and value, 
 * and decides whether it will execute a named or anonymous 
 * script. Calls either execute_script() or execute_named_script().
 */
functionName () {}

Which is better than:

/**
 * This function parses through a message that is given to it
 * and decides what to do based on what information it is given.
 */
whatEvenIsThisFunction() {}

Functions

Name functions as verbs if possible, in camelCase() for JavaScript and snake_case() for Python. Each function should do exactly what its name implies that it does, to avoid any confusion. Stay away from vague function names like parseString(). What is it parsing? Maybe something like parseMovementCommand() is better.

Additionally, one function should be responsible for one task and not seven. That's what helper functions are for! Hooray for reusable code!

Example:

// BAD FUNCTION
/**
 * Loads all components onto page, then adds event listeners.
 */
loadPage() {
  // la la stuff about document.ready or something
  // la la injecting things into HTML
  // la de da adding event listeners
}

// GOOD FUNCTION
/**
 * Adds HTML components to the page on document load.
 */
addComponentsToPage() {
  // Adds components to page
}

/**
 * Adds event listeners to form elements on index page.
 * Executed on load, within main().
 */
initEventListeners() {
  // Add event listeners w00
}

/**
 * Main function executed on page load. Adds necessary
 * HTML components to page, then registers event listeners.
 */
main() {
  addComponentsToPage();
  initEventListeners();
}

Variables

Naming

Please name them properly. Variables like name should be used with discretion (only if it makes sense in context, so that you can avoid being too vague), and certainly stay away from single letter variables unless it's something like a coordinate (x, y, or z). Though even then, maybe something like xLoc, yLoc, zLoc might be better.

Typing (this is more of a js thing)

var makes me uncomfortable lmao, so since we're using ES6 let's use it to its full might!!!! Unless you're going to be really imposing on the rest of the codebase and make a super visible/global variable, try to stay away from var. If you're never going to change the value of a variable, use const (highly suggested). Otherwise, let is a great one too. Let's not overstep scope.

New file? Same file???

If a group of functions serves one very specific purpose that may have more complex components added later, it's probably a good idea to put it in a new file. Just remember to add the right references.

Also, when you're working with front-end, make sure that things are as modular as possible, and that there aren't any scripts in the HTML page, etc. etc.