-
Notifications
You must be signed in to change notification settings - Fork 0
Common Code Requirements
Ben Buie edited this page May 14, 2024
·
3 revisions
This file is a work in progress and we welcome additions/edits. The goal here is to include best practices that make code more readable and user friendly. This is by no means an extensive list, but it does include a lot of helpful tips.
-
Don't confuse
- Always think through what future developers will think when seeing your code
- Simple code always wins. If it is too confusing, you may have to simplify.
-
Self documenting code
- The general rule is that your code should be human readable. It should read like a book, but a book of code.
- In a perfect world, every line of code should be understandable even without seeing much of its context
- This isn't always possible, but to the extent we can, we do
- Variables/properties/function argument names should represent what they are
- Recommend
var permissionsCommaSeparated = 'string'
notvar p = 'string'
- Recommend
- Boolean variables/properties/function argument should be named as questions or as statements
- Recommend
var isLoadingSomething = true
notvar spinner = true
- Recommend
var codeHasBeenApplied = true
notvar applied = true
- The goal is to make logical statements more readable
- Recommend
if(codeHasBeenApplied){
vsif(s.applied){
- Recommend
- Recommend
- Function names should describe what they do.
- Recommend
function getSomethingSpecific(){}
notfunction s(){}
- Avoid anonymous functions whenever possible.
- Recommend
- Complex associative arrays need to be given a less complex name
- Recommend
var someObject = someArray[someOtherArray[index]]; someObject.push;
notsomeArray[someOtherArray[index]].push()
- Recommend
- Logical statement should always be readable
- Recommend
var noCompaniesFound = city['west_side'].length < 0; if(noCompaniesFound){...}
- Recommend
-
Don't leave commented code
- In otherwords, leave unused code in a codebase because you're worried about losing some previous logic
- The only reason to leave commented code is if you're only commenting it temporarily
- If you do leave commented code, leave a comment why and the issue number related to the change
-
Comments in code
- If you leave a to do comment or other explanation comment in your code, please also leave an issue number
- Although there are sometimes where you'd want to leave some documentation as a comment, this should be rare. Comments often become outdated and confusing to future developers. Instead, if you're tempted to leave a comment, you should first think of ways you could document that logic within the property/method/function/variable names.
- There are some rare cases when short comments are needed, but these should be the exception
-
Code should be readable
- First comes first
- All exports or anything returned by a function should be visible within the first several lines of a file/function
- If something is run immediately when a file is loaded, it needs to be close to the top
- All variable declarations should be declared as close to where they are used first as possible
- Note, this is a departure from some common code practices where people like to list all the variables at the top of a function. To us, context and readability is more important.
- Functions should be defined after they are called
- The first functions in a file should be the ones that call other functions
- In other words, if you read a whole file from top to bottom, you should never wonder, "what function is dependent on this function?" because if you read that a function is called on line 10, you'll always know that the function definition is declared on line > 10.
- Correct indentation should be used
- Anything that has a an opening and closing (html tags, functions, etc.) should have all contents indented
- Indentation method should match throughout entire file
- It doesn't matter if the file uses 2 spaces, 4 spaces, or tabs for indentation. The important thing is that it matches.
- Whenever possible, use 4 spaces as the default and avoid tabs.
- Most indentation issues happen because of text editor settings
- If using sublime text editor you may want to make sure the default setting
"translate_tabs_to_spaces": false,
isn't overriden.
- First comes first
-
Long lines should be broken up by line breaks
- The longer a line gets the harder it is to read quickly and understand
- Lines with a lot of logic are more likely to cause merge conflicts
- Object keys should each have their own line
- Look for opportunities to add line breaks, particularly in html
-
Avoid merge conflicts
- Touch as few lines of code in a file as possible
- The more lines you touch, the more likely another developer has touched the same line.
- Touch as few lines of code in a file as possible
- Branching
- Every branch name should include an issue id
- There is no reason to create a pull request without documentation behind why that pull request is needed.
- Issues should only have one branch
- The code to resolve an issue should be all part of one branch
- This will make it easier to handle, trace, and merge in the future
- This will also prevent changes from being lost from development to staging and production
- Every branch name should include an issue id
- Use
forEach
overfor
to loop through arrays of items- Recommend
arr.forEach(eachItem)
notfor(i; i<arr.length;i++){}
- Although forEach is less efficient, it is more readable and satisfactory for most use cases
- If you absolutely must use for(...){}, make sure to assign the iterated variables first thing withing the for enclosure
- Recommend
for(...){var item = items[i]; item.push(newItem))
notfor(...){items[i].push(newItem))
- Recommend
- Recommend
- Avoid pyramid code
- Don't use anonymous functions in callbacks
- Recommend
Something.find(...).exec(findSomethingCallback)
notSomething.find(...).exec(function(){})
)
- Recommend
- Use promises in favor of callbacks
- Don't use anonymous functions in callbacks
- Avoid switch
- Most problems that tempt you to use a switch can be more robustly handled by an object and an if statement
- There are some exceptions, but always try to make the code as readable as possible
- If a switch does seem like the best tool for your problem, then encapsulate the inside of a case with a function.
- Code organization
- Contollers/Directives need to have a setup/init function where all controller variables and any functions that are run immediately are declared.
function setup(){ vm.someArray = []; getSomeData(); }
- Contollers/Directives need to have a setup/init function where all controller variables and any functions that are run immediately are declared.
- Create/Update Sub Data
- Sub data is data that may be stored in a different table or collection but that can be updated on the save of a parent.
- Sub data that is changed should be loaded via an id and saved on parent update
- Sub data is removed should be removed instantly via its api (unless it doesn't have an id and then it is deleted instantly from the view)
- Elements with lots of attributes need line breaks
- Some elements in angular become hard to read because of multiple attributes (e.g.
ng=*
or others). - Recommend breaking long elements up with line breaks between every attribute
- Some elements in angular become hard to read because of multiple attributes (e.g.
- User driven design
- Buttons
- Button Feedback
- A user should never have to wonder if their action was received
- If a user clicks a button for an asynchronous action, the user should be shown a spinner or loader immediately
- Prevent double clicks
- The instant a button is clicked, it should be disabled or hidden to prevent double clicks
- Button Feedback
- Errors/Warnings/Success Feedback
- All actions should give both success or error feedback
- Sometimes this feedback is that they see their desired change take place (e.g. like a box is hidden)
- If there is no visual feedback (like on save) then they should be shown a success message
- All feedback should appear close to where the user took the action
- Avoid alerts that show at the top of the page when a button was pressed at the bottom
- Make sure a user can't miss the feedback
- All actions should give both success or error feedback
- Buttons
- Model/View/Controller Structure
- Controllers
- Controllers should primarily handle requests and build responses.
- The logic in controllers should mainly be related to requests and responses.
- Skinny controllers are preferred.
- Payloads are typically sent to the model mergeOrCreate function to create an instance or update an existing instance.
- If a request needs data from a third party api or resource, the controller should use a service to interact with the resource.
- Models
- Any logic related to pulling data from the database or updating data should be found here
- On smaller projects, we also include business logic here. On larger projects, we break the business logic out into different classes
- Views
- Views on most of our projects are handled by a JavaScript framework and should not be built in Laravel.
- Service
- A service is basically a class that we can use to separate third party logic or other self contained logic from the models or controllers
- The most common use case for a service is interacting with a third party api. We'd use the service to document the structure and functions necessary to get the data we want.
- Controllers
- Media Queries
- Keep queries small
- Queries should be located very close to the styles they're overriding.
- Rather than have a long query with multiple classes and sub classes, have a short query within each class that overrides the related styles.
- Keep queries small
- Routes should be readable
- Don't put the method name in the url
- Recommend for GET method
'/api/something' to getSomething()
not'/api/something/getSomething' to getSomething()
- Recommend for GET method
- Avoid camelCase
- Web addresses are case insensitive so camel case may be readable in code, but it is confusing in the wild
- If you must use multiple words in a route, think about separating them by a
/
or a-
- Don't put the method name in the url