Naming functions or methods
I will show you bad and good naming of methods with the help of some code examples. Let’s see some of the typical functions related code smells on a case by case basis.
Smells —
Abbreviations
- appointCR(…) // What does CR mean? Appoint code reviewer or class responsible? It was about appointing a class responsible role to a teacher. Hence rename it to appointClassResponsible()
- handleClk() // This one is supposed to handle Clicked. So, why not rename it to handleClicked, onSaveClicked, onSave or onUpdate?
Not doing what it says
- changePage(url){ this.props.history.push(url); } // It was actually navigating to given url, so, I would rename it to navigateTo(url).
- validationErrors()// It was doing student validation. The Name doesn’t tell you anything. I would rename it to validateStudent.
Use of nouns instead of verbs in method names
- jwtAuthentication()
– I would rename it to a verb like authenticate() - jwtTokenDecryption()
– I would rename it to a verb like decryptToken() - jwtTokenValidation()
– I would rename it to a verb like validateToken() - categoryOptions()
– I would Rename it to a verb like getCategoryOptions()
Noisy words
- getAppointmentInfo()
- getAppointmentDetails()
- getAppointmentInformation()
Why are you adding noisy/redundant words like Info, Details, Information in front of appointment? In my opinion, getAppointment() gives us information about the appointment. Keep it simple.
Further,
- getStudentsList()
- getStudentsCollection()
- getStudentsArray()
Just getStudents() make sense to me. You don’t need to tell your audience about how’s your implementation under the hood.
Not symmetric
- turnOn()/disable()
- openConnection()/shutConnection()
- increaseVolume()/turndownVolume()
If you are adding symmetric methods, then make sure they feel symmetric one.
- turnOn()/turnOff()
- openConnection()/closeConnection()
- readFile()/writeFile()
- increaseVolume()/decreaseVolume()
Not having descriptive / searchable names
- booleanValue(value)?? Oh! After browsing the code, I realized that it was converting the string value to boolean. I would rename it to toBoolean(value)
- deactivate() : It was deactivating the membership. I would rename it to deactivateMembership()
- invoke() : It was invoking a controller’s action. I would rename it to invokeAction().
- disable() : Disable what? Authorization? I would rename it to disableAuthorization().
- handle() : Handling what? Handling subscription? I would rename it to handleSubscription().
One word per concept
- fetchActiveStudents()
- retrieveActiveStudents()
- getActiveStudents()
Why are you using different words (fetch, retrieve, get) for the same concept? Just choose one word per concept and use it throughout your project.
- getActiveStudents()
- getPackages()
- getAppointments()
Side effects
- ValidateToken() should NOT create a token. I would take `create token` logic out of the method. It’s a side effect.
- getValue(key, callbackForCreate) should not create value for a given key. Sometimes, people try to get a value from the cache for a
given key. If the value is absent then they try to create value inside the cache using a given callback function. I would separate these concerns into different functions. Read more about command query separation. - createUser() — This method was doing multiple things. It was validating user inputs, creating a user, sending registration email using asynchronous operation. This method seems like registerUser() and it should outsource the responsibilities to validateUser(), createUser() that stores a record and sendEmailAsync() method that conveys that email would be sent later.
If you are not going to refactor it into three new methods due to the deadline, then at least rename it to registerUserAndSendEmail()
Anti-patterns:`And`, `Or`, `If`.
- createUserAndSignToken()
- createOrUpdateUser()
If a method contains And, Or, If in its name, it’s an indication that the method is doing more than one thing, which is a violation of Single Responsibility Principle.
Command and query in one function
- fetchAndSaveCategoryDetails() method is not just doing more than one thing, but also executing command and query in one function. The best thing is to simply separate command and query. Create separate functions. fetchCategory() and saveCategory().
The guidelines for naming methods:
- FUNCTIONS SHOULD DO ONE THING. THEY SHOULD DO IT WELL. THEY SHOULD DO IT ONLY… Clean code by Uncle Bob.
- The function should be cohesive. The function should say what you mean. The consumer of the function should not browse your function to see what it is doing.
- Separate command and query: Create a separate function for command and query. Here, command means to save, update, delete the records and query means all get or list methods like getStudent(id), getActiveStudents() etc.
- Explore open source projects for your technologies and explore how names are given. I explored many JavaScript libraries, Lavavel PHP source code, .NET source code: https://referencesource.microsoft.com/
Originally published at https://beingcraftsman.com on January 20, 2019.