Refactoring

Any fool can write code that a computer can understand - Good programmers write code that humans can understand

Martin Fowler

...and why

return new GeolocationResponseData(0, 0, 0, 0);

is not a good API

eyalmrejen@gmail.com

Great code reads like great prose. It is succinct, expressive, and clear the first time you read it. It tries to be as linear as possible, guiding the reader through tough transitions with the knowledge that one wrong move could lose them entirely. Good code uses language and vocabulary with an understanding of its audience

Matthew Bischoff

https://matthewbischoff.com/code-is-prose/

Based On

This guy...

Martin Fowler (born 1963) is a British software developer, author, and international public speaker on software development, specializing in object-oriented analysis and design, UML, patterns, and agile software development methodologies, including extreme programming.

His 1999 book Refactoring popularised code refactoring. In 2018, a 2nd edition, this time in JavaScript, was published.

Agenda

What is Refactoring
Why Refactor
What, When, How?
Drawbacks and issues
Refactoring patterns

What is Refactoring?

Refactoring is the process of changing a software system in such a way that it does not alter the external behavior of the code yet improves its internal structure. It is a disciplined way to clean up code that minimizes the chances of introducing bugs. In essence when you refactor you are improving the design of the code after it has been written.

Martin Fowler

"Refactoring: Improving the Desing of Existing Code"

Why Refactor

At first...life is simple

// Function to calculate total ticket cost
function printTicketStatement(tickets) {
  let totalCost = 0;

  for (const ticket of tickets) {
    let thisCost = 0;
    // Determine cost based on ticket type
    switch (ticket.getType()) {
      case Ticket.STANDARD:
        thisCost += 10; // Base price for standard ticket
        if (ticket.getQuantity() > 3) {
          thisCost += (ticket.getQuantity() - 3) * 8;
        }
        break;
      case Ticket.VIP:
        thisCost += ticket.getQuantity() * 25;
        break;
    }
    totalCost += thisCost;
  }

  return `Total cost is $${totalCost.toFixed(2)}`;
}

A few months later...

// Function to calculate total ticket cost with discount
function printTicketStatement(tickets) {
  let totalCost = 0;
  let totalTickets = 0; // Accumulator for total quantity of tickets

  // Step 1: Calculate base cost and total ticket count
  for (const ticket of tickets) {
    let thisCost = 0;
    const quantity = ticket.getQuantity();
    totalTickets += quantity; // Add to accumulator

    switch (ticket.getType()) {
      case Ticket.STANDARD:
        thisCost += 10; // Base price
        if (quantity > 3) {
          thisCost += (quantity - 3) * 8;
        }
        break;
      case Ticket.VIP:
        thisCost += quantity * 25;
        break;
      case Ticket.PREMIUM: // New type
        thisCost += quantity * 40; // $40 per Premium ticket
        break;
    }
    totalCost += thisCost;
  }

  // Step 2: Apply discount based on total ticket count
  let discountPercent = 0;
  if (totalTickets >= 10) {
    discountPercent = 20; // 20% off for 10+ tickets
  } else if (totalTickets >= 5) {
    discountPercent = 10; // 10% off for 5-9 tickets
  } else if (totalTickets >= 3) {
    discountPercent = 5;  // 5% off for 3-4 tickets
  }

  const discountAmount = totalCost * (discountPercent / 100);
  const finalCost = totalCost - discountAmount;

  return `Total cost is ${finalCost.toFixed(2)} (Original: ${totalCost.toFixed(2)}, ${discountPercent}% discount applied)`;
}

From this

To this

And then you want
- ```
printStatmentAsHtml()
```
You are in a hurry
Copy & Paste to the Rescue
Yea...we all did that...

Yes, but...

This is a stupid duplication...Nobody writes like this, right?
So, what's the problem?

https://gitlab..../cismanager/service/impl/UserPermissionServiceImpl.java

Expectations

"Nobody," writes code like that
Your code is (probably) great
Code, like we saw, emerges after a collaboration
Accumulated modifications lead to this code

Reality

Others alter your code
You alter other people's code
This is good - collaboration is good
Code is begin constantly revisited
External conditions affect quality of work

Expectations

This code can appear in suboptimal conditions
This is expected in an agile process

Reality

Agility means very small upfront design - the simplest thing that can work

Code Evolves (Darwin Nov 24, 1859)

Cosmic Constant - it's harder to maintain code than to write a new one
- Most Developers hope to write a greenfield project
- NIH Syndrom
Actuality - this what we do most of the time
A good advice - KISS

Why Refactor?

Improves the Design of a software
Makes Software Easier to Understand
Helps to find bugs
Helps me to program faster

What?, When?, How?

When Should We Refactor?

Constantly (this means all the time - don't tell me this is old code...)
The Rule of Three
- "Three strikes and you refactor" - https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)
Preparatory Refactoring - Make it easier to add a feature
Comprehension Refactoring - Make code easier to understand

When Should we Refactor?

Litter - Pickup Refactoring
Planned and Opportunistic Refactor
Boy Scout principle

Why fix it, if it ain't broken?

Every module has three functions
- To execute according to its purpose
- To afford change
- To communicate it's intent to the reader
If it does not do one or more of these - it is broken

function MM_validateForm() { //v4.0
  var
i,p,q,nm,test,num,min,max,errors='',args=MM_validateForm.arguments;
  for (i=0; i<(args.length-2); i+=3) { test=args[i+2]; val=MM_findObj(args[i]);
    if (val) { nm=val.name; if ((val=val.value)!="") {
      if (test.indexOf('isEmail')!=-1) { p=val.indexOf('@');
        if (p<1 || p==(val.length-1)) errors+='- '+nm+' must contain an e-mail address.\n';
      } else if (test!='R') { num = parseFloat(val);
        if (isNaN(val)) errors+='- '+nm+' must contain a number.\n';
        if (test.indexOf('inRange') != -1) { p=test.indexOf(':');
          min=test.substring(8,p); max=test.substring(p+1);
          if (num<min || max<num) errors+='- '+nm+' must contain a number between '+min+' and '+max+'.\n';
    } } } else if (test.charAt(0) == 'R') errors += '- '+nm+' is required.\n'; }
  } if (errors) alert('The following error(s) occurred:\n'+errors);
  document.MM_returnValue = (errors == '');

When NOT to Refactor?

When I see ugly code, but for now I don't really need to understand it...leave it for when you must
When it's easier to re-write than refactor
YAGNI - "You ain't gonna need it" - When it's premature
When fixing a bug - fix first, only then refactor if needed
When tests are failing

Code Smells

Code Smells - "If it stinks, change it"

https://en.wikipedia.org/wiki/Code_smell

With time, code that isn't "updated" - decays
So, what constitutes a code smell?
- Is any feature in the source code of a program that might hint at a larger problem (Wikipedia)
Test code can indeed decay

Code Smells Examples

Magic numbers
Duplicate code
Long Method
Complicated Conditions
Large Functions/Classes

Code Smells

https://en.wikipedia.org/wiki/Code_smell

Code Smells

https://blog.codinghorror.com/code-smells/

Change Preventers

These smells mean that if you need to change something in one place in your code, you have to make many changes in other places too. Program development becomes much more complicated and expensive as a result.

In/Between classes

A change leads to Divergent change
- - Divergent change occurs when you have to do too many changes in a class/module to introduce a new feature/change. If you're doing this, you're almost certainly violating the principles of one key abstraction and separation of concerns
  - Extract Class - Superclass / Subclass

Code Smell - Examples

class Product {
    private type: string;

    constructor(type: string) {
        this.type = type;
    }

    public getBasePrice() : number {
        switch (this.type) {
            case 'food':
                return 10;
            case 'drinks':
                return 7;
            case 'books':
                return 3;
            default:
                return 0;
        }
    }

    public getTaxPercent() : number {
        switch (this.type) {
            case 'food':
            case 'drinks':
                return 24;
            case 'books':
                return 8;
            default:
                return 0;
        }
    }

    public getProductCategory(): string {
        switch (this.type) {
            case 'food':
            case 'drinks':
                return 'Food and Beverages';
            case 'books':
                return 'Education';
            default:
                return '-';
        }
    }
}

class Product {
    private type: string;

    constructor(type: string) {
        this.type = type;
    }

    public getBasePrice() : number {
        switch (this.type) {
            case 'food':
                return 10;
            case 'drinks':
                return 7;
            case 'books':
                return 3;
            case 'vitamins':
                return 1;
            default:
                return 0;
        }
    }

    public getTaxPercent() : number {
        switch (this.type) {
            case 'food':
            case 'drinks':
                return 24;
            case 'books':
                return 8;
            case 'vitamins':
                return 3;
            default:
                return 0;
        }
    }

    public getProductCategory(): string {
        switch (this.type) {
            case 'food':
            case 'drinks':
                return 'Food and Beverages';
            case 'books':
                return 'Education';
            case 'vitamins':
                return 'Pharmaceutical';
            default:
                return '-';
        }
    }
}

class Product {
    private type: string;
    static productTypes: { [key: string]: ProductInfo } = {
        food: {
            tax: 24,
            basePrice: 10,
            productCategory: 'Food and Beverages'
        },
        drinks: {
            tax: 24,
            basePrice: 7,
            productCategory: 'Food and Beverages'
        },
        books: {
            tax: 8,
            basePrice: 3,
            productCategory: 'Education'
        },
    }

    constructor(type: string) {
        this.type = type;
    }

    public getBasePrice() : number {
        return Product.productTypes[this.type].basePrice || 0;
    }

    public getTaxPercent() : number {
        return Product.productTypes[this.type].tax || 0;
    }

    public getProductCategory(): string {
        return Product.productTypes[this.type].productCategory || '-';
    }
}

https://avraam.dev/posts/divergent-change

A change leads to Shotgun surgery

Making any modifications requires that you make many small changes to many different classed
Causes
- Not maintaining separation of concerns
- Not respecting the single responsibility principle (S.O.L.I.D)
Fix
- Move Method,
- Move a field
- Inline class

Between classes

public class Account {

    private String type;
    private String accountName;
    private int amount;
    
    public Account (String  type, String accountName, int amount) {
        this.type = type;
        this.accountName = accountName;
        this.amount = amount;
    }

    public void debit(int debit) throws Exception {
        if(amount < 500) throw new Exception("Minimum balance should be over 500");
        amount = amount - debit;
    }

    public void transfer(Account from, Account to, int creditAmount) throws Exception {
        if(from.amount < 500) throw new Exception("Minimum balance should be over 500");
        to.amount = amount - creditAmount;
    }

    public void sendWarningMessage() throws Exception {
        if(amount < 500) 
        	eventBroker.publishWarningMessage("Minimum balance should be over 500")
    }
}

Refactor Move To Method: isAccountUnderFlow()

Divergent Change resembles Shotgun Surgery but is actually the opposite smell. Divergent Change is when many changes are made to a single class. Shotgun Surgery refers to when a single change is made to multiple classes simultaneously.

Code Smell - Comments


//The class definition for Account
class Account {
  private profit: number = 0;
  //constructor
  constructor() {
    
  }
  //set the profit member to a new value
  setProfit(profit: number) {
    this.profit = profit;
  }
  //return the profit from this account
  getProfit() {
    return this.profit;
  }
}

All comments here are worthless

Code Smell - Comments

//Check to see if the employee is eligible for full benefits
if(employee.flags & HOURLY_FLAG && (employee.age > 65)) {
    //
}


if(employee.isEligibleForBenefits()) {
    //
}

Code Smell - Comments


        this.byteOPos = writeBytes(pngIdBytes, 0);
        //hdrPos = bytePos;
        writeHandler();
        writeResolution();
        //dataPos = bytePos;
        if(writeImageData()) {
            writeEnd();
            this.pngBytes = resizeByArray(this.pngBytes, this.maxPos);
        }

https://refactoring.guru/smells/comments

Code Smell - Todo

Code Smell - Magic Numbers

A magic number is a numeric literal (8080, 200, 404, 5, 8) that is used in the middle of a block of code without explenation

double potentialEnergy(double mass, double height) {
    return mass * height * 9.81;
}

    public static final double GRAVITATIONAL_CONSTANT = 9.81;
    
    double potentialEnergy(double mass, double height) {
        return mass * height * GRAVITATIONAL_CONSTANT;
    }

https://www.codereadability.com/magic-numbers/

  const sentences = nums.reduce((acc, curr) => {
    switch (curr) {
      case 96:
      case 50:
      case 37:
      case 2:
        acc.add('The page includes abnormal/broken DOM elements')
        break
      case 14:
      case 31:
        acc.add('The page contain abnormal/broken HTML attributes')
        break
      case 4:
        acc.add('The page does not use the best practices for securing sensitive inputs')
        break
      case 34:
        acc.add('The page utilizes suspicious page execution data')
        break
      case 108:
        acc.add('The page contains unnecessary different origin data')
        break
      case 1:
        acc.add('This page domain and URL are suspicious')
        break
      default:
        break
    }
    return acc
  }, new Set<string>())

Magic Numers

Magic Strings....are the same issue

Code Smell - When is a function too long

https://stackoverflow.com/questions/475675/when-is-a-function-too-long

   /**
     * This method will validate the given device association details.
     * In case the validation fails, an exception will be thrown with a relevant error message.
     * @param workspace - the shop workspace
     * @param deviceAssociationDto - shop device DAO
     * @param action - indicates which action type is handled
     * @param accountId - the id of the account
     * @param customerRefIdsForAccount
     * @throws ValidationException
     */
    private List<ValidationExceptionMessages> validateAction(Long workspace, DeviceAssociationDetailsDto deviceAssociationDto, DeviceAssociationSaveAction action, String accountId, List<String> customerRefIdsForAccount) {


        SimpleDateFormat simpleDateFormat = new SimpleDateFormat(DATE_FORMAT);
        List<ValidationExceptionMessages> validationExceptionMessages = new ArrayList<>();

        if (action != null && action.equals(DeviceAssociationSaveAction.RMA_ASSIGN_DEVICE)) {
            return validationExceptionMessages;
        }

        if (StringUtils.isEmpty(deviceAssociationDto.getSerial())) {
            validationExceptionMessages.add(ValidationExceptionMessages.SERIAL_IS_MANDATORY);
        }


        if(StringUtils.isNotEmpty(deviceAssociationDto.getSerial()) && !verifySerialExistInCustomer(accountId, deviceAssociationDto.getSerial(), customerRefIdsForAccount)){
            validationExceptionMessages.add(ValidationExceptionMessages.INVALID_SERIAL);
        }

//        if (StringUtils.isEmpty(deviceAssociationDto.getShopCode()) && deviceAssociationDto.getSubRegionId()==null && deviceAssociationDto.getRegionId()==0) {
        if (deviceAssociationDto.getEntityId() == 0) {
            validationExceptionMessages.add(ValidationExceptionMessages.ILLEGAL_REGION); // At least region id should be filled
        }

        try {

            if (StringUtils.isEmpty(deviceAssociationDto.getFrom())) {
                validationExceptionMessages.add(ValidationExceptionMessages.FROM_DATE_MANDATORY);
            } else if(!TimeUtils.isValidDateFormat(deviceAssociationDto.getFrom(), DATE_FORMAT)) {
                validationExceptionMessages.add(ValidationExceptionMessages.INVALID_DATE_FORMAT);
            } else if(action.equals(DeviceAssociationSaveAction.ADD_ASSIGNMENT_HISTORY)){
                Date from = simpleDateFormat.parse(deviceAssociationDto.getFrom());
                Date to = simpleDateFormat.parse(deviceAssociationDto.getTo());
                if(!(from.before(new Date()) || from.equals(new Date()))) {
                    validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_HISTORY_CAN_NOT_BE_IN_THE_FUTURE);
                }
                if (to.before(from)) {
                    validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_HISTORY_TO_BEFORE_FROM);
                }
            }

            // Partial mandatory fields validations, if mandatory field is missing --> no need to continue the validation
            if (!validationExceptionMessages.isEmpty()) {
                return validationExceptionMessages;
            }

            CustomerElementType elementType = CustomerElementType.valueOf(deviceAssociationDto.getAssignmentEntityType());
            if(elementType.equals(CustomerElementType.REGION) && deviceAssociationDto.getEntityId()==0) {
                validationExceptionMessages.add(ValidationExceptionMessages.ILLEGAL_REGION);
            } else if(elementType.equals(CustomerElementType.SUB_REGION) && deviceAssociationDto.getEntityId()==0) {
//                Long shopIdByCode = organizationStructureService.getShopIdByCode(deviceAssociationDto.getShopCode(), workspace);
//                if (StringUtils.isNotEmpty(deviceAssociationDto.getShopCode()) && (shopIdByCode == null)) {
                    validationExceptionMessages.add(ValidationExceptionMessages.SHOP_CODE_NOT_EXIST);
//                }
            }

            DeviceDto device = deviceService.getDeviceBySerial(deviceAssociationDto.getSerial());
            if (device == null) {
                validationExceptionMessages.add(ValidationExceptionMessages.DEVICE_ASSOCIATION_NOT_EXIST);
            }

            if(action.equals(DeviceAssociationSaveAction.ADD_ASSIGNMENT_HISTORY)){
                if(StringUtils.isEmpty(deviceAssociationDto.getTo()))  {
                    validationExceptionMessages.add(ValidationExceptionMessages.TO_DATE_MANDATORY);
                } else if(!TimeUtils.isValidDateFormat(deviceAssociationDto.getTo(), DATE_FORMAT)) {
                    validationExceptionMessages.add(ValidationExceptionMessages.INVALID_DATE_FORMAT);
                }else {
                    Date to = null;
                    if (StringUtils.isNotEmpty(deviceAssociationDto.getTo())) {
                        to = simpleDateFormat.parse(deviceAssociationDto.getTo());
                    }
                    if(!(to.before(new Date()) || to.equals(new Date()))) {
                        validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_HISTORY_CAN_NOT_BE_IN_THE_FUTURE);
                    }
                }
            }

            List<DeviceAssociation> deviceAssociationHistories = accountDeviceDao.getDeviceAssociationHistory(accountId, deviceAssociationDto.getSerial());

            if (CollectionUtils.isNotEmpty(deviceAssociationHistories)) {

                Date from = simpleDateFormat.parse(deviceAssociationDto.getFrom());

                Date to = null;
                if (StringUtils.isNotEmpty(deviceAssociationDto.getTo())) {
                    to = simpleDateFormat.parse(deviceAssociationDto.getTo());
                }

                for (DeviceAssociation deviceAssociationHistory : deviceAssociationHistories) {

                    // Avoid comparing range to itself
                    if(deviceAssociationHistory.getId() == deviceAssociationDto.getId()) {
                        continue;
                    }

                    // Verify there is no conflict with current shop assignment
                    if(deviceAssociationHistory.getToDate()==null && to!=null) {
                        // [from]=currentAssignmentFrom
                        if (from.equals(deviceAssociationHistory.getFromDate())) {
                            validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_ALREADY_EXIST_FOR_DEVICE);
                        }
                        // [to]>=currentAssignmentFrom
                        if (to.equals(deviceAssociationHistory.getFromDate()) || to.after(deviceAssociationHistory.getFromDate())) {
                            validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_ALREADY_EXIST_FOR_DEVICE);
                        }
                    }

                    // [from]=rangeFrom or [from]=rangeTo
                    if (from.equals(deviceAssociationHistory.getFromDate()) || from.equals(deviceAssociationHistory.getToDate())) {
                        validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_ALREADY_EXIST_FOR_DEVICE);
                    }

                    // Verify there is no conflict with history shop assignment
                    if (deviceAssociationHistory.getToDate() != null && to != null) {

                        // [to]=rangeFrom or [to]=rangeTo
                        if (to.equals(deviceAssociationHistory.getFromDate()) || to.equals(deviceAssociationHistory.getToDate())) {
                            validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_ALREADY_EXIST_FOR_DEVICE);
                        }

                        // rangeFrom < [from] < rangeTo
                        if (from.after(deviceAssociationHistory.getFromDate()) && from.before(deviceAssociationHistory.getToDate())) {
                            validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_ALREADY_EXIST_FOR_DEVICE);
                        }

                        // rangeFrom < [to] < rangeTo
                        if (to.after(deviceAssociationHistory.getFromDate()) && to.before(deviceAssociationHistory.getToDate())) {
                            validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_ALREADY_EXIST_FOR_DEVICE);
                        }

                        //  [from] < rangeFrom and rangeTo < [to]
                        if (from.before(deviceAssociationHistory.getFromDate()) && to.after(deviceAssociationHistory.getToDate())) {
                            validationExceptionMessages.add(ValidationExceptionMessages.ASSIGNMENT_ALREADY_EXIST_FOR_DEVICE);
                        }
                    }

                }
            }
        } catch (ParseException e) {
            e.printStackTrace();
        }

        return validationExceptionMessages;
    }

Code Smell - boolean flags

https://www.codereadability.com/boolean-arguments/

user.setDisabled(true);
user.setDisabled(false);
//
settings.createSetting("userAge", true);
settings.createSetting("userAge", false);

//
//https://angular.io/api/forms/ControlValueAccessor
form.setDisabledState(true);
form.setDisabledState(false);

user.enable();
user.disable();

//Chrome DevTools source 
createLocalSetting: function(name){
    this.createSetting(name, true)
}

settings.createLocalSetting("userAge");

Code Smell - When we have a complicated condition

if((now() > months[3]) && (now() < months[10])) {
    //
}

if(isWinter(now())) {
    //
}

const monthOf => index => months[index];
const March = 3;
const October = 10;

if((now() > monthOf(March)) && (now() < monthOf(October))) {
    //
}

Code Smell - When we have a complicated condition

req.body && Object.keys(req.body).length > 0 && req.body.keyId && req.body.iv && req.body.payload

if(isRequestBodyEncrypted()) {
    //
}

Code Smell - Code Duplication

https://refactoring.guru/smells/duplicate-code

Code Smell - Code Duplication

Still counts - even though it's not exactly duplication

https://refactoring.guru/smells/duplicate-code

Code Smell - Large Class

https://exceptionnotfound.net/god-objects-the-daily-software-anti-pattern/

Failure of the Single Responsibility Principle

Feature Envy - A method accesses the data of another object more than its own data
God class/Object - A class or other object knows too much or does too much, and is therefore difficult to test or change

public class Employee
{
    public int ID { get; set; }
    public DateTime BirthDate { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string SpouseFirstName { get; set; }
    public string SpouseLastName { get; set; }
    public string ChildOneFirstName { get; set; }
    public string ChildOneLastName { get; set; }
    ...
}

Refactoring Patterns

Refactoring Catalog

https://refactoring.com/catalog/

Extract Method

  if(sentence.toLowerCase().lastIndexOf(badWord) === sentence.length - badWord.length) {
    return ThreatLevel.MEDIUM;
  }

  if(badWordIsAtTheEnd(badWord, sentence)) {
    return ThreatLevel.MEDIUM;
  }

Pull Up Method

class Employee {...}

class Salesman extends Employee {
  get name() {...}
}

class Engineer extends Employee {
  get name() {...}
}

class Employee {
  get name() {...}
}

class Salesman extends Employee {...}
class Engineer extends Employee {...}

Introduce Parameter Object

function amountInvoiced(startDate, endDate, priority) {...} //1st time
function amountReceived(startDate, endDate, priority) {...} //again
function amountOverdue(startDate, endDate, priority) {...} // and again

//define "factorish" function
const aDateRageWithPriority = (startDate, endDate, priority) => ({startDate, endDate, priority});

//invoke
const range = aDateRageWithPriority(startDate, endDate, priority);

function amountInvoiced(range) {...}
function amountReceived(range) {...}
function amountOverdue(range) {...}

Remove Flag Argument

//PLEASE NOTE THAT THIS FUNCTION DOES 2 THINGS...
function setDimension(name, value) {
  if (name === "height") {
    this._height = value;
    return;
  }
  if (name === "width") {
    this._width = value;
    return;
  }
}

function setHeight(value) {this._height = value;}
function setWidth (value) {this._width = value;}

Replace Parameter with Explicit Methods

Use Selectors

//code has intimate knowledge of the person data structure
const manager = aPerson.department.manager;
//or
const {department: {manager}} = aPerson;


//extract (selector) function
const selectManagerFrom = aPerson => aPerson.department.manager

//now use it cleanly
const manager = selectManagerFrom(aPerson);

//Also please look at the style of english, how more fluent it is,
//than rather coding - no getManager() for example
//Code should be written like prose

How to Refactor

The sloppy way - doing it manually

https://www.youtube.com/watch?v=W8LLUIIwSBw

The Copy & Paste way

https://www.youtube.com/watch?v=3GK2JckFX5g

The Best way - use the IDE

https://www.youtube.com/watch?v=z_C-CBoTtx0

Summary

Code rots
Refactoring improves existing code
Refactoring improves maintainability
Refactor continuously
Iterative Process
- Test pass
- Refactor
- Repeat

Resources

Refactoring by Martin Fowler
- 1st addition - Java
- 2nd addition - Javascript
Smells to Refactorings Quick Reference Guide
Refactoring Guru
Javascript Code Readability
The poetics of code - smashing magazing

Refactoring

By Eyal Mrejen

Refactoring

Based On

Agenda

What is Refactoring?

What is Refactoring?

Why Refactor

At first...life is simple

A few months later...

Yes, but...

Expectations

Reality

Expectations

Reality

Code Evolves (Darwin Nov 24, 1859)

Why Refactor?

Why Refactor?

What?, When?, How?

When Should We Refactor?

When Should we Refactor?

Why fix it, if it ain't broken?

When NOT to Refactor?

Code Smells

Code Smells - "If it stinks, change it"

Code Smells Examples

Code Smells

Code Smells

Change Preventers

In/Between classes

Code Smell - Examples

Between classes

Code Smell - Comments

Code Smell - Comments

Code Smell - Comments

Code Smell - Comments

Code Smell - Todo

Code Smell - Magic Numbers

Code Smell - When is a function too long

Code Smell - boolean flags

Code Smell - When we have a complicated condition

Code Smell - When we have a complicated condition

Code Smell - Code Duplication

Code Smell - Code Duplication

Code Smell - Large Class

Refactoring Patterns

Refactoring Catalog

Extract Method

Pull Up Method

Introduce Parameter Object

Remove Flag Argument

Use Selectors

How to Refactor

Summary

Resources

Refactoring

More from Eyal Mrejen