Bringing Conceptual Integrity to Legacy Software

Let's talk about maintainability

Maintainability

"Maintainability focuses on the ease with which a software system can be maintained. Maintenance of a software system tasks place as changes are made to it after the software is in operation. Maintenance is necessary to preserve the value of the software over time" - Software Architect's Handbook, Joseph Ingeno, 2018

 

 

Software Design Stamina Hypothesis

Text

https://martinfowler.com/bliki/DesignStaminaHypothesis.html

const createUser = (data) => {
  const db = new Postgresql(process.env.POSTGRES_URI);
  const payload = {};
  payload.id    = data.id;
  payload.name  = data.name;
  payload.email = data.email;

  const Result = db.insert('User', payload);
  for (let i = 0; i < 3; i++) {
    try {
      await axios.post('https://api.mailgun.com/api/v1/emails', {
        to: Result.email,
        content: 'Welcome... to Jamaica!'
      });
      break;
    } catch (error) {
      console.log('retrying');
    }
  }

  firebase.sendPushNotification(data.userDevice, Result);

  // Notifies microservice A
  new Kafka(process.env.kafkaBroker).sendMessage('userCreated', Result);

  // Notifies microservice B
  await RPC.notify('userCreated', Object.assign({}, Result));
};

Is this code maintainable?

import CreateUser from '@/usecases/createUser'

describe('Create User', () => {
  it('should create a user', () => {
    try {
      const command = new CreateUser();

      command.execute();
    } catch (error) {
      expect(error).to.not.exist;
    }
  });
});

Is this test even worth it?

What are the issues with that code?

  • Too many responsibilities
  • No error handling
  • Coding style inconsistencies
  • Coupling with external modules

Can this code become maintainable?

  • Decoupling the Use Case from implementation details.
  • Automated Linters

Should it be more maintainable?

It depends.

One approach

What did we achieve?

  • Decoupled Mailing and Push Notification providers from business logic, giving the possibility to the codebase to support more providers for emails and notifications.
  • Abstracted the data access mechanism with Repository Pattern

No Silver bullets

Let's talk about Conceptual Integrity


Conceptual Integrity is the principle that anywhere you look in your system, you can tell that the design is part of the same overall design. This includes low-level issues such as formatting and identifier naming, but also issues such as how modules and classes are designed, etc.

https://blogs.agilefaqs.com/2009/02/01/conceptual-integrity/


Lack of consistency can lead to poor communication and complex client code. Having a consistent style of design is the very essence of Conceptual Integrity.

 

Some times despite high cohesion and low coupling, the system might not have conceptual integrity. This is because the overall style, themes, mood, does not tie it all together.

https://blogs.agilefaqs.com/2009/02/01/conceptual-integrity/

// 1) src/utils/sendEmail.js

import mailgunClient from 'mailgun-js'

const mailgun = mailgunClient.createInstance({ 
  apiKey: process.env.API_KEY 
})

const sendEmail = (payload) => {
  try {
    var emailContent = {}
    if (payload.to) {
      emailContent.to = payload.to
    }
    if (payload.content) {
      emailContent.html = payload.content
    }

    mailgun.send(emailContent)

    return { status: 'ok' }
  } catch (error) {
    if (error.type === "EXPIRED_CREDENTIALS")
      return { status: 'expired_credentials' }
    return { status: 'error' }
  }
}
// 2) src/mailing/MailgunSender.js

import mailgunClient from 'mailgun-js';

class MailgunError < Error;
class MailgunExpiredCredentials < MailgunError;

class MailgunSender {
  constructor (apiKey) {
    this.apiKey = apiKey;
    this.client = mailgunClient.createInstance({
      apiKey 
    });
  }

  send({ to, body }) {
    try {
      this.client.send({ to, body });
    } catch (error) {
      if (error.type === 'EXPIRED_CREDENTIALS') {
        throw new MailgunExpiredCredentials();
      }

      throw new MailgunError();
    }
  }
}

Imagine both in the same codebase

Is there anything wrong? Both source codes make exactly the same

Another Example: Data Access Layer Concepts

How can we achieve conceptual integrity?

Some ideas

  • Using a unified code styleguide.
  • Gather the most common concepts like Store, Gateway, Repository, Container, Component and give them a very explicit meaning so it's intuitive to use them for refactoring, hoping to unify the whole code at some point
  • Design software before implementing via technical specs, UML diagrams.
  • Having a responsible for the overall system design.
  • Constantly monitor the execution of the software with Code Reviews, Pair Programming and metrics.
  • Do not try to fix everything at once.
  • Be consistent.
  • Communication is critical

Effects of conceptual integrity

  • Overhead trying to understand the different ways of making a same thing (like fetching data from a database) will be removed.
  • The team will not be able to tell who wrote what line of code as the team writes consistent code.
  • Onboarding new members to the team gets faster as everyone lives a unified style of coding

Legacy code and conceptual integrity

Legacy Code

Strict Definition: Legacy code is code that we have gotten from someone else

Legacy code is simply code without tests.

Working Effectively with Legacy Code. Michael Feathers. 2004

Software that defines the roadmap of a product and the agenda of a team by developing the Stockholm Syndrome on everyone inside and outside the development team, including stakeholders and users by making itself just too hard to change and extend.

Manipulative Software

Common problems with Legacy

  • Rigidity: Code being difficult to change (coupling)
  • Fragility: A change tend to break unrelated functionality to the changed code.
  • Hidden intentions behind spaguetti code
  • Multiple levels of abstraction in a single class or function.
  • Bad or inexistent test suites.

Some things we might do

  • Analyze the system use cases.

  • Identify how those use cases map to the system.

  • Prioritize the most critical parts of the system that need the most corrective maintenance.

  • Design a plan to gradually refactor if possible.

Progressive Refactoring

Keeps adding value to the business, slowly but more effective if done well.

  • Refactor inside the code base
  • Refactor outside the code base

Complete re-write

  • Try to avoid as much as possible.
  • There's no value added to the business until the re-write is complete.
  • Re-write could take months or years if not well planned.
  • Risk of falling into the second system effect.

Some things that might help

  • Clean Architecture
  • Test Driven Development (Unit/Integration testing)
  • Design Patterns
  • CI/CD

Clean Architecture

Test Driven Development

Using Facade Pattern

class NotificationsFacade {
  constructor(
    emailSender, 
    notificationsGateway
    deviceStore
  ) {
    this.emailSender = emailSender;
    this.notificationsGateway = notificationsGateway;
    this.deviceStore = deviceStore;
  }

  sendWelcomeEmail (user) {
    const email = new Email({
      to: user.email,
      from: 'awesome@service.io',
      concept: 'Welcome to Jamaica!',
      body: 'This is awesome, thanks for joining'
    });

    this.emailSender.send(email);
  }

  sendWelcomeNotification (user) {
    const device = this.deviceStore.findByUserId(
      user.id
    );
    const notification = new PushNotification({
      title: 'Welcome to Jamaica!',
      content: 'This is awesome, thanks for joining'
    });

    this.notificationsGateway.send(
      device, notification
    );
  }
}

class CreateUserLegacy {
  execute () {
    // thousands of repetitive lines of code
    notificationsFacade.sendWelcomeEmail(user);
  }
}
  • Creates an interface for a complex subsystem
  • Connects legacy to the interface. Legacy knows nothing about implementation details.

Characterization Tests

A characterization test is a test that characterizes the actual behavior of a piece of code. There’s no “Well, it should do this” or “I think it does that.” The tests document the actual current behavior of the system.

const sumOne = (number) => {
  return number * 42432 / (42431 + 0.5 + 0.2 + 0.3) + 3;
};

describe('sumOne', () => {
  it('Sums three to my number', () => {
    expect(sumOne(1)).to.equal(4);
  });
});

Conclusion

  • Dealing with legacy code is about attitude.
  • There are already defined strategies to gradually fix conceptual integrity in legacy software.
  • Bringing conceptual integrity to legacy software involves Software Design, Code Style, Low-level agreements.
  • Communication is key.
  • Enjoy the Journey.

Some Material

Clean Architecture by Robert C. Martin

Working Effectively with Legacy Code by Michael Feathers

Mythical Man-Month by Frederick Brooks

Some Material

Code Simplicity by Max Kanat-Alexander

Test-Driven Development by Example by Kent Beck

Alberto Romero

Software Engineer @Rever

 

twitter: @aromeronavia

Thanks a lot

Made with Slides.com