Full Stack Developer at Intrasoft International S.A.
Software Engineer
& Aspiring Craftsman
You know you are working on clean code when each
routine you read turns out to be pretty much what
you expected.Ward Cunningham
int d; ///elapsed time in days
int elapsedTimeInDays;
Intention revealing names
class DtaRcrd102 {
private Date genymdhms;
private modymdhms;
/* .... */
}
class Customer {
private Date generationTimestamp;
private Date modificationTimestamp;
/* .... */
}
Pronounceable names
ClassifierNumber classifierString; //name not changed when type changed!!
public interface ProcessI {
default void print(){
System.out.println("Item processed");
}
}
Hungarian notation
CustomBigDecimal totalAmount = new CustomBigDecimal(250.0);
CustomBigDecimal totalAmount = CustomBigDecimal.fromRealNumber(250.0);
Method names
Student student = new Student.Builder(1L)
.firstName("Alexis")
.lastName("Pavlidis")
.semester(8)
.address("20 Georgiou 8")
.email("apavlidi@it.teithe.gr")
.phoneNumber(6999999L)
.registeredDate(LocalDate.of(1996, 10, 9)).build();
Student student = new Student(1L,"Alexis","Pavlidis",8,
"20 Georgiou 8","apavlidi@it.teithe.gr",
6999999L,LocalDate.of(1996, 10, 9))
Method names Pt.2
public String add(String firstName, String lastName){
return firstName + " : " + lastName;
}
public void add(ProductItem item){
this.productItems.add(item);
}
public void add(String firstName, String lastName){
return firstName + " : " + lastName;
}
public void insert(ProductItem item){
this.productItems.add(item);
}
Don`t pun
getActiveAccount();
getActiveAccountInfo();
BigDecimal amount;
BigDecimal amountMoney;
Make meaningful distinctions
public class CustomerInfo {
/* .... */
}
public class CustomerData {
/* .... */
}
Rules of functions:
Do one thing
FUNCTIONS SHOULD DO ONE THING ONLY
public void emailClients(List<Client> clients) {
for (Client client : clients) {
Client clientRecord = repository.findOne(client.getId());
if (clientRecord.isActive()){
email(client);
}
}
}
public void emailClients(List<Client> clients) {
for (Client client : clients) {
if (isActiveClient(client)) {
email(client);
}
}
}
private boolean isActiveClient(Client client) {
Client clientRecord = repository.findOne(client.getId());
return clientRecord.isActive();
}
Do one thing!
public void DoStuff(string email, string userName, string country, string city, string street, string complement, string phone)
{
/* .... */
}
public void DoStuff(User user, Address address)
{
/* .... */
}
Function arguments: Objects
static int calculateInvoices(int multiplier, BigDecimal... invoices) {
int totalBalance = 0;
for (int invoice : invoices){
totalBalance += invoice * multiplier;
}
return totalBalance ;
}
Function arguments : Lists
public Booking book(Customer aCustomer, boolean isPremium) {
if(isPremium)
// logic for premium book
else
// logic for regular booking
}
public class Concert {
public Booking regularBook(Customer aCustomer) {
// logic for regular book
}
public Booking premiumBook(Customer aCustomer) {
// logic for regular book
}
}
Flag arguments
public class UserValidator {
private Cryptographer cryptographer;
public boolean checkPassword(String userName, String password) {
User user = UserGateway.findByName(userName);
if (user != User.NULL) {
String codedPhrase = user.getPhraseEncodedByPassword();
String phrase = cryptographer.decrypt(codedPhrase, password);
if ("Valid Password".equals(phrase)) {
Session.initialize();
return true;
}
}
return false;
}
}
Have no side effects
If you need comments to understand what the code is doing then the code is poorly written
/**
MIT License
Copyright (c) 2018 Alexis Pavlidis
Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
© 2019 GitHub, Inc.
*/
Legal comments (Good)
// Don't run this if you're short on time!
public void testFullRequestCycle(){
}
// This will overwrite production data...
public function seedDatabase(){
}
Warning comments (Good)
public static void hireEmployee(Employee employee) {
//TODO Validate employee before persisting to DB
EmployeeDAO employeeDAO = new EmployeeDAO();
employeeDAO.saveEmployee(employee);
}
TODO/FIXME comments (Good)
/**
* Zaps the machine with the number of volts you specify.
* <p>
* Do not exceed more than 30 volts or the zap function will backfire.
*
* @exception IOException if you don't enter a data type amount for the voltage
* @param voltage the number of volts you want to send into the machine
* @see Dynamite#blowDynamite
*/
public void zapMachine(int voltage) throws IOException {
if (voltage < MAXIMUM_VOLTAGE) {
System.out.println("Zapping machine with " + voltage + " volts!!!!");
} else {
System.out.println("Backfire!!! zapping machine with 1,000,000 volts!!!!");
}
}
Javadocs in Public APIs (Good)
Use variables/functions instead of comments (Bad)
// if the user is not the author of the announcement or if he is not an admin
// he can`t edit the announcement so throw an exception
if (!(announcement.getPublisher().getId() === req.getUser().getId()
|| req.getUser().getEduPersonScopedAffiliation() === config.PERMISSIONS.admin)){
throw new AuthorizationException("No permission to edit the announcement!");
}
if (userIsNotThePublisher(announcement, req.getUser()) && userIsNotAnAdmin(req.getUser()){
throw new AuthorizationException("No permission to edit the announcement!");
}
private boolean userIsNotThePublisher(Announcement announcement, User user){
return !announcement.getPublisher().getId().equals(user.getUser().getId());
}
private boolean userIsNotThePublisher(User user){
return !req.getUser().getEduPersonScopedAffiliation().equals(config.PERMISSIONS.admin));
}
private int bonus(String allRoles, String currentRole, int index) {
int nextRollScore = scoreForRoll(getNextRoll(allRoles, index));
if (isLastFrame(allRoles, index))
return 0;
// if (currentRole.equals(SPARE_SYMBOL))
// return nextRollScore;
if (currentRole.equals(STRIKE_SYMBOL)) {
String nextRoleString = getNextRoll(allRoles, index + 1);
return nextRollScore + scoreForRoll(nextRoleString) -
spareDiff(allRoles, nextRoleString, index + 2);
}
return 0;
}
Comment out code (Bad)
Newspaper Metaphor
public class WikiPage {
private PageData pageData;
private PageCrawler crawler;
private List<Article> articles;
/* .... */
public Item pop(){
// logic here
}
public void add(){
// logic here
}
public void print(){
// logic here
}
/* .... */
}
Conceptual affinity
public class CustomerValidator {
public boolean checkPassword(String userName, String password){
Optional<User> user = userRepository.findByUsername(userName);
if(user.isPresent()){
return isPasswordValid(password);
}
return false;
}
private boolean isPasswordValid(String password){
return checkIfPasswordFollowsRules(password) &&
checkIfPasswordIsReused(password);
}
private boolean checkIfPasswordFollowsRules(String password){
// logic here
}
private boolean checkIfPasswordIsReused(String password){
// logic here
}
}
Dependent functions
Team rules