Skip to main content

Best Software Development Practices

Our primary goal is to write code that is maintainable, readable, and easy to understand. Clear code reduces debugging marathons and lets us focus on the fun part: developing new features.

A Note on Reality: We know that fully adhering to these principles isn't always possible—especially with low-level hardware drivers or math-heavy algorithms. However, we aim for "best effort" whenever practical.

Visit C++ or ROS2 for more related best practices.

Adhere to SOLID Principles​

Acronym for five object-oriented design guidelines:

  • Single Responsibility Principle
  • Open-Closed Principle
  • Liskov Substitution Principle
  • Interface Segregation Principle
  • Dependency Inversion Principle

Single Responsibility Principle​

A class or function should have only one reason to change.

Problem: A Student class is responsible for storing student data, persisting it to a database, sending emails, and handling course enrollment.

class Student {
public:
    std::string getFirstName();
    std::string getLastName();
    std::string getEmail();

    void save() {
        // Code to save student to database
    }
    void email(std::string subject, std::string body) {
        //Code to email a student
    }

    void enrol(Course course) {
        // code to enrol the student in a course
    }

};

Fix: Split them. Let Student hold data, a StudentRepository handle the database, and a Mailer handle emails. Smaller, focused functions improve readability, simplify testing, and reduce the likelihood of overly complex god classes or excessively large functions.

Open-Closed Principle​

Code should be open for extension, but closed for modification. You should be able to add new features without rewriting existing code.

Problem: Using a massive if/else block to check for different shapes. Adding a "Triangle" means editing the original class.

class AreaCalculator {
public:
    double calculate(std::string shape) {
        if (shape == "circle") {
            // calculate circle area
        } else if (shape == "rectangle") {
            // calculate rectangle area
        }
        // To add a triangle, you must modify this class
    }
};

Fix: Use Polymorphism. Create a base Shape class with a virtual area() method.

class Shape {
public:
    virtual double area() = 0;
};

class Circle : public Shape {
public:
    double area() override { /* calculate circle area */ }
};

class Rectangle : public Shape {
public:
    double area() override { /* calculate rectangle area */ }
};

// To add a triangle, just create a new class — no existing code is touched
class Triangle : public Shape {
public:
    double area() override { /* calculate triangle area */ }
};

Liskov Substitution Principle​

You should be able to replace a base class with any of its subclasses without breaking the app.

Problem: If Human has a cook() method, but the Child subclass can't cook, you've broken the program's logic.

Fix: Move cook() to a Parent class instead of the generic Human class.

Interface Segregation Principle​

Don't force a class to implement methods it doesn't use. Smaller, specific interfaces are better than one "fat" interface.

Problem: A BasicPrinter can only print, but since it inherits from IPrinter, it is forced to implement scan() and fax() even though it does not support them

class IPrinter {
public:
    virtual void print() = 0;
    virtual void scan() = 0;
    virtual void fax() = 0;
};

// A basic printer can only print, but is forced to implement Scan and Fax
class BasicPrinter : public IPrinter {
public:
    void print() override { /* print */ }
    void scan() override { throw "Not supported!"; }
    void fax() override { throw "Not supported!"; }
};

Fix: Instead of one IPrinter, create three separate interfaces. A basic printer only inherits IPrinter.

class IPrinter {
public:
    virtual void print() = 0;
};

class IScanner {
public:
    virtual void scan() = 0;
};

class IFax {
public:
    virtual void fax() = 0;
};

// Basic printer only implements what it needs
class BasicPrinter : public IPrinter {
public:
    void print() override { /* print */ }
};

// All-in-one printer implements everything it supports
class AllInOnePrinter : public IPrinter, public IScanner, public IFax {
public:
    void print() override { /* print */ }
    void scan() override { /* scan */ }
    void fax() override { /* fax */ }
};

Dependency Inversion Principle​

High-level logic shouldn't depend on low-level tools. They should both depend on abstractions (interfaces).

Problem: A NotificationService (high-level) directly depends on EmailSender (low-level). This means if we ever want to add SMS notifications, we would have to modify NotificationService.

// Low-level module
class EmailSender {
public:
    void send(std::string message) {
        // Code to send an email
    }
};

// High-level module directly depends on a low-level module
class NotificationService {
    EmailSender email_sender;
public:
    void notify(std::string message) {
        email_sender.send(message);
    }
};

// To add SMS notifications, you would have to modify NotificationService

Fix: Introduce an IMessageSender interface. Both EmailSender and SmsSender implement it, and NotificationService depends on the interface rather than any concrete class. This way, new notification methods can be added without touching any existing code.

// Abstraction
class IMessageSender {
public:
    virtual void send(std::string message) = 0;
};

// Low-level modules depend on the abstraction
class EmailSender : public IMessageSender {
public:
    void send(std::string message) override {
        // Code to send an email
    }
};

class SmsSender : public IMessageSender {
public:
    void send(std::string message) override {
        // Code to send an SMS
    }
};

// High-level module also depends on the abstraction, not a concrete class
class NotificationService {
    IMessageSender& sender;
public:
    NotificationService(IMessageSender& sender) : sender(sender) {}

    void notify(std::string message) {
        sender.send(message);
    }
};

Beware the "Iceberg Class"​

An Iceberg Class is one where most of the logic is hidden in private methods, since only the tip of the class is visible from the outside.

Problem: When you feel tempted to test a private method, it is usually a sign that the class has too many responsibilities.

Fix: Consider extracting those private methods into a separate class where they become public and can be tested directly.

Simplify Your Arguments​

Functions should ideally take no more than 2–3 arguments. The same idea applies to return values.

Problem: A fuction createUser(first_name, last_name, email, age, address) has too many arguments, making it hard to read.

// Hard to read, hard to understand what each value represents
std::vector<std::pair<std::pair<char, int>, std::vector<int>>> getResults();

void createUser(std::string first_name, std::string last_name, 
                std::string email, int age, std::string address);

Fix: Wrap them in a struct UserInfo. It’s cleaner to read and easier to extend later.

struct UserInfo {
    std::string first_name;
    std::string last_name;
    std::string email;
    int age;
    std::string address;
};

void createUser(UserInfo user);

Meaningful Code > Comments​

Before writing a comment, ask: "Can I make the code so clear that a comment is useless?"

Problem:

// Check if user is an adult
if (user.age >= 18) { ... }

Fix:

if (user.isAdult()) { ... }

Avoid Comment Banners​

Avoid using giant comment headers to separate code sections.

Problem: A method class constructor is so long it needs visual divisions.

FancyClass::FancyClass(){
    // --------- ROS parameters ---------
    param_loader_->loadParam("dummy_parameter_1", _dummy_parameter_1_);
    param_loader_->loadParam("dummy_parameter_2", _dummy_parameter_2_);
    ...

    // --------- ROS interfaces ---------
    // subscribers
    subscriber_ = this->create_subscription<std_msgs::msg::String>(
        "topic", 10, std::bind(&FancyClass::topic_callback, this, _1)
        );
    
    // publishers
    publisher_ = this->create_publisher<std_msgs::msg::String>("topic", 10);
}

Fix: Extract those sections into private helper methods like initRosInterfaces() or loadParams(). This makes the main logic a high-level "table of contents."

FancyClass::FancyClass(){
    loadParams_();

    initRosInterfaces_();
}

void FancyClass::loadParams_(){
    ...
    param_loader_->loadParam("dummy_parameter", _dummy_parameter_);
}

void FancyClass::initRosInterfaces_(){
    initRosSubscribers_();
    initRosPublishers_();
}

void FancyClass::initRosSubscribers_(){
    subscriber_ = this->create_subscription<std_msgs::msg::String>(
        "topic", 10, std::bind(&FancyClass::topic_callback, this, _1)
        );
}

void FancyClass::initRosPublishers_(){
    publisher_ = this->create_publisher<std_msgs::msg::String>("topic", 10);
}

One Class, One File​

Try to keep each class in its own file and avoid putting everything into a single file.

Problem: While convenient at first, many classes in one file become difficult to navigate and maintain as the codebase grows.

Fix: Split each class to its own file. It also makes collaboration easier, as multiple people can work on different files without conflicts.

Deep Dive Resources​

If you want to master the "why" behind these rules, we highly recommend: