Bad or not bad code
Introduction
Andrii Zhukov
Senior Software Engineer
at GlobalLogic
Information for you
You can support our defenders with the help of
QR code for the purchase of
the thermal imager model
HikMicro GRYPHON GH25 or
its equivalent.
This fundraise is a personal
initiative and the Company
has neither organized, nor
is the beneficiary of the
fundraising.
Information for you
This fundraise is a personal initiative and the Company
has neither organized, nor is
the beneficiary of the
fundraising.
PrivatBank card:
5168752017879597
Жуков Андрій Валерійович
Agenda
- What is clean code?
- What about memory leaks?
- Solid, Dry, Kiss, Yagni, etc.
- A set of young developer tools to help us
- This hard choice of naming
- Use good architectural solutions
- Make the code cleaner
- Don't forget about optimization
- Doing a code review
What is clean code?
JavaScript memory model
Mark and sweep
Memory leaks
Global variables: without key word, using var, etc.
Use strict mode
'use strict';
window.user = null;
// or
this.user = null;Memory leaks
setInterval, addEventListener, file descriptors, sockets, etc.
clearInterval, removeEventListener, close all descriptors.
Memory leaks
Closures remember the surrounding context and can store large objects
function outer() {
const bigArray = [];
return () => {
for (let i = 0; i < 1000000; i++) {
bigArray.push(i);
}
console.log('doing something');
};
}
const closureFunc = outer();
closureFunc();
// some code
closureFunc();Memory leaks
DOM elements and their referrences
const allComments = [];
function removeComment() {
const postComments =
document.getElementById(
'post-comments'
);
allComments.push(postComments);
document.body.removeChild(postComments);
}
removeComment();Memory leaks
Other ways to leak memory
Cache
Map -> WeakMap
Observers
WeakRef
FinalizationRegistry
Solid
Single Responsibility Principle
export class DoEverything {
showComment() {...}
hideComment() {...}
saveComment() {...}
findEmailInComment() {...}
replaceUrlInComment() {...}
}export class Comment {
show() {...}
hide) {...}
save() {...}
}
export class SomeLib {
findEmail() {...}
replaceUrl() {...}
}Solid
Open Closed Principle
const posts = [
new Post('iPhone'),
new Post('BMW'),
new Post('Asus')
];
for(let i = 0; i < posts.length; i++) {
switch(posts[i].getCategory()) {
case 'car':
console.log('logic for car');
break;
case 'phone':
console.log('logic for phone');
break;
case 'laptop':
console.log('logic for laptop');
break;
}
}Solid
class Post {
doSomeLogic() {
console.log('basic logic');
}
}
class CarPost extends Post {
doSomeLogic() {
console.log('logic for car');
}
}
class PhonePost extends Post {
doSomeLogic() {
console.log('logic for phone');
}
}
class LaptopPost extends Post {
doSomeLogic() {
console.log('logic for laptop');
}
}const posts = [
new CarPost(),
new PhonePost(),
new LaptopPost()
];
for(let i=0; i < posts.length; i++){
posts[i].doSomeLogic();
}Open Closed Principle
Solid
Liskov Substitution Principle
class Shape {
get area() {...}
}
class Rectangle extends Shape {
get area() {...}
}
class Square extends Shape {
get area() {...}
}
class Circle extends Shape {
get area() {...}
}const shapes = [
new Rectangle(1, 2),
new Square(1, 2),
new Circle(2),
]
for (let s of shapes) {
console.log(s.area);
}Solid
Interface Segregation Principle
class IPhone extends Phone {
call() { /*Implementation*/ }
publish() { /*Implementation*/ }
}class Phone {
constructor() {
if (
this.constructor.name ===
'Phone'
) {
throw new Error(
'Phone class is absctract'
);
}
}
call() {
throw new Error(
'call is absctract'
);
}
publish() {
throw new Error(
'publish is absctract'
);
}
}Solid
Interface Segregation Principle
class IPhone extends Phone {
call() { /*Implementation*/ }
}class Phone {
constructor() {
if (
this.constructor.name ===
'Phone'
) {
throw new Error(
'Phone class is absctract'
);
}
}
call() {
throw new Error(
'call is absctract'
);
}
}Solid
Dependency Inversion Principle
class PersistanceManager {
saveData(db, data) {
if (db instanceof FileSystem) {
db.writeToFile(data)
}
if (db instanceof ExternalDB) {
db.writeToDatabase(data)
}
if (db instanceof LocalPersistance) {
db.push(data)
}
}
}class FileSystem {
writeToFile(data) {
// Implementation
}
}
class ExternalDB {
writeToDatabase(data) {
// Implementation
}
}
class LocalPersistance {
push(data) {
// Implementation
}
}Solid
Dependency Inversion Principle
class PersistanceManager {
constructor() {
this.db = new FileSystem();
}
saveData(data) {
this.db.save(data)
}
}class FileSystem {
save(data) { /* Implementation*/ }
}
class ExternalDB {
save(data) { /* Implementation*/ }
}
class LocalPersistance {
save(data) { /* Implementation*/ }
}
class PersistanceManager {
saveData(db, data) {
db.save(data)
}
}class PersistanceManager {
constructor(Store) {
this.db = new Store();
}
saveData(data) {
this.db.save(data)
}
}Dry
Don't Repeat Yourself
console.log('I will not repeat my code');
console.log('I will not repeat my code');
console.log('I will not repeat my code');
console.log('I will not repeat my code');
console.log('I will not repeat my code');
console.log('I will not repeat my code');
console.log('I will not repeat my code');
Kiss
Keep It Short and Simple
Yagni
You Aren't Going to Need It
Necessary tools to help us
Code formatting
Eslint
Prettier
Unit testing
Style guide, manifest to code, etc
Correct naming
Your code should be read like a book
DarthVader
BadUser
U
Sr
SomePrefixUser
U1
UserInfo or UserData
UserDiffuser
UserAbuser
UserLoser
UserBoozer
Correct naming
What are the correct names?
Correct naming
What are the correct names?
Classes, variables are nouns
Functions, methods are verbs
Classes are PascalCase
Variables, functions are camelCase
Using one notation in the team
Constants are UPPER_CASE_SNAKE_CASE
Use of keywords
Clear definition
Using one entity
Don't use the cases described earlier
Good architectural solutions
What other solutions can we use?
OOP
Software Design Patterns
MVC
GRASP, etc
Make the code cleaner
Many developers think when the program works their work is over
Write code so that it is cleaner than it was before
Don't hardcode
Write readable code
Add spaces, indents, etc.
Remove commented code
Remove dead code, unless of course you get paid for it
Make the code cleaner
How can we do it?
git (github, gitlab, etc.)
prettier
eslint-config-airbnb, eslint-config-standard, etc.
eslint-config-prettier
eslint
Make the code cleaner
How can we do it?
Divide into separate logical blocks
Remove duplicate code
Move the code into constants, libraries, etc.
Using new ES6+ features:
- Rest, spread
- Default arguments value, etc.
Make the code cleaner
someFunction (proportionalTax, fiscal, controlling, regulating) {
const calculateTax = document.getElementById(('calculate-tax');
const salary = Number(document.getElementById('salary').textContent);
const comment = document.getElementsByClassName('comment', 'class');
calculateTax.innerHTML = proportionalTax + ':' +
'<br/>' + salary * .5 +
'<br/>Some information:<br/>' +
'- ' + fiscal + '<br/>' +
'- ' + controlling + '<br/>' +
'- ' + regulating;
Make the code cleaner
const getHtmlElement = (htmlElement, type = 'id') => {
if (type === 'id') {
return document.getElementById(htmlElement);
} else if (type === 'class') {
return document.getElementsByClassName(htmlElement);
} else if (type === 'tag') {
return document.getElementsByTagName(htmlElement);
}
// or
return document.querySelector(htmlElement);
};
someFunction (proportionalTax, fiscal, controlling, regulating) {
const calculateTax = getHtmlElement('calculate-tax');
const salary = Number(getHtmlElement('salary').textContent);
const comment = getHtmlElement('comment', 'class');
calculateTax.innerHTML = proportionalTax + ':' +
'<br/>' + salary * .5 +
'<br/>Some information:<br/>' +
'- ' + fiscal + '<br/>' +
'- ' + controlling + '<br/>' +
'- ' + regulating;
}Make the code cleaner
const getHtmlElement = (htmlElement, type = 'id') => {
if (type === 'id') {
return document.getElementById(htmlElement);
} else if (type === 'class') {
return document.getElementsByClassName(htmlElement);
} else if (type === 'tag') {
return document.getElementsByTagName(htmlElement);
}
// or
return document.querySelector(htmlElement);
};
someFunction (proportionalTax, fiscal, controlling, regulating) {
const calculateTax = getHtmlElement('calculate-tax');
const salary = Number(getHtmlElement('salary').textContent);
const comment = getHtmlElement('comment', 'class');
calculateTax.innerHTML = `${proportionalTax}:
${salary} * ${.5}
Some information:
- ${fiscal}
- ${controlling}
- ${regulating}`;
}Make the code cleaner
const getHtmlElement = (htmlElement, type = 'id') => {
if (type === 'id') {
return document.getElementById(htmlElement);
} else if (type === 'class') {
return document.getElementsByClassName(htmlElement);
} else if (type === 'tag') {
return document.getElementsByTagName(htmlElement);
}
// or
return document.querySelector(htmlElement);
};
someFunction (...taxKeywords) {
const [proportionalTax, fiscal, controlling, regulating] = taxKeywords;
const calculateTax = getHtmlElement('calculate-tax');
const salary = Number(getHtmlElement('salary').textContent);
const comment = getHtmlElement('comment', 'class');
calculateTax.innerHTML = `${proportionalTax}:
${salary} * ${.5}
Some information:
- ${fiscal}
- ${controlling}
- ${regulating}`;
}Make the code cleaner
const TAX_PERCENTAGE = .5;
const htmlElementTypes = {
id: 'id',
class: 'class',
tag: 'tag'
}
const getHtmlElement = (htmlElement, type = 'id') => {
const { id, class, tag } = htmlElementTypes;
if (type === id) {
return document.getElementById(htmlElement);
} else if (type === class) {
return document.getElementsByClassName(htmlElement);
} else if (type === tag) {
return document.getElementsByTagName(htmlElement);
}
// or
return document.querySelector(htmlElement);
};
someFunction (...taxKeywords) {
const [proportionalTax, fiscal, controlling, regulating] = taxKeywords;
const calculateTax = getHtmlElement('calculate-tax');
const salary = Number(getHtmlElement('salary').textContent);
const comment = getHtmlElement('comment', 'class');
calculateTax.innerHTML = `${proportionalTax}:
${salary} * ${TAX_PERCENTAGE}
Some information:
- ${fiscal}
- ${controlling}
- ${regulating}`;
}Make the code cleaner
// constants.js
export const TAX_PERCENTAGE = .5;
// libs.js
const htmlElementTypes = {
id: 'id',
class: 'class',
tag: 'tag'
}
export const getHtmlElement = (htmlElement, type = 'id') => {
const { id, class, tag } = htmlElementTypes;
if (type === id) {
return document.getElementById(htmlElement);
} else if (type === class) {
return document.getElementsByClassName(htmlElement);
} else if (type === tag) {
return document.getElementsByTagName(htmlElement);
}
// or
return document.querySelector(htmlElement);
};
// app.js
import { TAX_PERCENTAGE } from 'constants.js';
import { getHtmlElement } from 'libs.js';
someFunction (...taxKeywords) {
const [proportionalTax, fiscal, controlling, regulating] = taxKeywords;
const calculateTax = getHtmlElement('calculate-tax');
const salary = Number(getHtmlElement('salary').textContent);
const comment = getHtmlElement('comment', 'class');
calculateTax.innerHTML = `${proportionalTax}:
${salary} * ${TAX_PERCENTAGE}
Some information:
- ${fiscal}
- ${controlling}
- ${regulating}`;
}Make the code cleaner
// constants.js
export const TAX_PERCENTAGE = .5;
// libs.js
const htmlElementTypes = {
id: 'id',
class: 'class',
tag: 'tag'
}
export const getHtmlElement = (htmlElement, type = 'id') => {
const { id, class, tag } = htmlElementTypes;
if (type === id) {
return document.getElementById(htmlElement);
} else if (type === class) {
return document.getElementsByClassName(htmlElement);
} else if (type === tag) {
return document.getElementsByTagName(htmlElement);
}
// or
return document.querySelector(htmlElement);
};
// app.js
import { TAX_PERCENTAGE } from 'constants.js';
import { getHtmlElement } from 'libs.js';
someFunction (taxKeywords) {
const { proportionalTax, fiscal, controlling, regulating } = taxKeywords;
const calculateTax = getHtmlElement('calculate-tax');
const salary = Number(getHtmlElement('salary').textContent);
const comment = getHtmlElement('comment', 'class');
calculateTax.innerHTML = `${proportionalTax}:
${salary} * ${TAX_PERCENTAGE}
Some information:
- ${fiscal}
- ${controlling}
- ${regulating}`;
}Make the code cleaner
Comments in code
It's better to remove comments for the code
Сode must be self-documented
It's bad when a comment requires a comment to its comment
Remove commented code
Make the code cleaner
Classes in code
class Post {
name = 'Default post';
title;
description;
#id;
constructor() { ... }
set postInfo(post) {
const {title, description} = post;
if (title && description) {
this.title = title;
this.description = description;
}
}
get postInfo() {
return `Title: ${this.title}, description: ${this.description}`;
}
getName() {
return this.name;
}
setName(newName) {
this.name = newName;
}
getId() {
return this.#id;
}
#setId() {
this.#id = Math.random();
}
}Make the code cleaner
Methods in code
No more than 10 methods per class
The contents of the method must fit on the screen
Next method below where it is called
Short method name
Division of responsibility
Make the code cleaner
Methods in code
First declare the variables then the rest of the code
Limited declaration of arguments in a function
Getter or setter
Pure functions
Don't forget about optimization
const sendActiveEmails = (customers) => {
customers.forEach((customer) => {
if (customer.status === 'active') {
sendEmail(customer);
}
});
};const sendActiveEmails = (customers) => {
customers.filter(isActiveCustomer)
.forEach(sendEmail);
};
const isActiveCustomer = (customer) => {
// some code
return customer.status === 'active';
};Don't forget about optimization
What else?
Simplify the complexity of the algorithm
Don't do an extra re-render of the HTML structure
Stop code execution where it's not needed
Doing a code review
What should you pay attention to?
Fixing memory leaks
Good architectural decisions must be applied
Use correct naming in code
Don't forget about code optimization
Apply basic clean code tips
Recommendation
Learn and apply best practices in your code
That's all
Questions
You can support our defenders with the help of
QR code for the purchase of
the thermal imager model
HikMicro GRYPHON GH25 or
its equivalent.
This fundraise is a personal
initiative and the Company
has neither organized, nor
is the beneficiary of the
fundraising.
Questions
This fundraise is a personal initiative and the Company
has neither organized, nor is
the beneficiary of the
fundraising.
PrivatBank card:
5168752017879597
Жуков Андрій Валерійович
Questions
You can visit GlobalLogic site and search JavaScript
positions with the help of
QR code:
Bad or not bad code
By andreyvalerievich
