Understanding SOLID and YAGNI Principles Through Discord Bot Refactoring

In this article, I'll delve into the SOLID and YAGNI design principles that I apply to refactor my Discord bot, a process that I document in my [Discord Bot Refactor] blog. In some examples, I will show code that the bot used to illustrate what you should avoid.

Breaking Down SOLID: Principles for Robust Software

The SOLID design principle is actually a collection of OOP(Object-Oriented Programming) principles that, when applied together, create readable, maintainable, and flexible software. SOLID consists of the following principles:

• S(SRP) Single Responsibility Principle
  means that a module shouldn't do more than one thing. For example, the class that handles CRUD operations for server data shouldn't also handle CRUD operations for user data.

    # This is a violation of SRP
  
    class MongoData(): # This is a bad name.
    # I won't list all methods of the class here since it has 400 lines of code
        def get_server(guild_id: int) -> dict:
            query = servers.find_one({"_id" : guild_id})
            return query
        # [...]
        def get_user(user_id:int) -> dict:
            query = users.find_one({"_id": user_id})
            return query
  
    # Instead the code should be split in two classes
  
    # One for Server CRUD
    class ServerDatabaseAdapter():
        def get_server(guild_id: int) -> dict:
            query = servers.find_one({"_id" : guild_id})
            return query
  
    # And one for User CRUD
    class UserDatabaseAdapter():
        def get_user(user_id:int) -> dict:
            query = users.find_one({"_id": user_id})
            return query
  

• O(OCP): Open-Closed Principle

  The OCP on the software design layer is pretty straightforward and just says that implemented modules like classes should be open for extension but closed for modification. For example, instead of adding functionality to an existing class, you should rather make a subclass of that class and implement your new functionality that way, even better if you abstract it further into an interface.

  If I wanted to add functionality to the MongoData class, I would have to just add it directly to the class, which would further bloat its responsibilities.

  The solution here would be to implement a general Interface for Databases and then implement that. That way I could have classes like the UserDatabaseAdapter from the SRP example that implement a basic Database interface which defines which methods the UserDatabaseAdapter should have, like create, read, update and delete.

• L(LSP): Liskow Substitution Principle

  This principle boils down to the fact that subclasses should always be able to be used in place of their base classes. LSP nowadays usually doesn't get violated on the software design layer since most languages enforce that you implement an interface correctly; however, it sometimes gets violated on the software architecture layer.
  A prominent example of that would be IQueryable, where LINQ to Entities and LINQ to OData aren't interchangeable

• I(ISP): Interface Segregation Principle

  It basically says that a client shouldn't be forced to depend on methods it doesn't use. Since ISP has OOP in mind, you will often hear that a consumer shouldn't depend on an interface that defines methods it doesn't use. We can still break this principle down to the MongoData class, though.
  ISP in my bot is violated, for example, by a module that only needs to interact with user data because it would still have to interact with the entire MongoData class, which includes all the other unrelated methods.

  Since a lot of people often don't understand the difference between SRP and ISP, I'm going to outline that with a quick example as well.

    from abc import ABC, abstractmethod
  
    class IPrinter(ABC):
        @abstractmethod
        def print_document(self, document):
            pass
  
    class IScanner(ABC):
        @abstractmethod
        def scan_document(self):
            pass
  
    class Printer(IPrinter):
        def print_document(self, document):
            print(f"Printing document: {document}")
  
    class Scanner(IScanner):
        def scan_document(self):
            print("Scanning document...")
            return "Scanned data"
  
    class MultiFunctionPrinter(IPrinter, IScanner):
        def print_document(self, document):
            print(f"Printing document: {document}")
  
        def scan_document(self):
            print("Scanning document...")
            return "Scanned data"
  

  Here we do have two interfaces, IPrinter and IScanner, with a simple print and scan method, and three classes, Printer, Scanner, and MultiFunctionPrinter, which implement the interfaces. We can clearly see that even though the interfaces and classes follow SRP, it wouldn't be enough to also be in line with the ISP. So ISP sometimes means that we end up with interfaces that are smaller than the classes that implement them, even if those classes follow SRP.

D(DIP): Dependency Inversion Principle
Let's start with the definition of dependency inversion, as it's used almost everywhere, including Wikipedia:

1. High-level modules should not import anything from low-level modules. Both should depend on abstractions.

2. Abstractions should not depend on details. Details (concrete implementations) should depend on abstractions.

First of all, we should define what exactly is meant by high- and low-level modules.
and once again, the most common occurrence of those in OOP are interfaces and classes. With an interface being the high-level modul and a class that implements that interface the low-level modul. Furthermore, only concrete implementations should know the details.
Now it slowly becomes clear that the DIP is focused on decoupling to segregate logic levels.

But to make it a bit easier:
A high-level module is mainly concerned with the questions "What needs to be done?" and "Why should it be done?" while a low-level module implements the "How is it done?" For example, a high-level module would know the business logic but no details about the implementation of it in the data access logic.

But here is an example of a violation of the DIP to really drive the point home:

# This is a command my discord bot has to change the port of the affiliated server
@commands.command(aliases=["chport"])
@commands.check(Checks.admin_check)
async def change_port(self, ctx, port:int):
    try:
        if int(port):
            MongoData.update_server_value(ctx.guild.id, key="server_port", value=int(port))
            await ctx.send(f"I changed your port to {port}!")
    except ValueError:
        await ctx.send(f"{port} is not a number!")

# And here is the implementatiotn of the MongoData.update_server_value() method
def update_server_value(guild_id: int, key: str, value) -> None:
    servers.update_one({"_id" : guild_id}, {"$set":{key : value}})

The change_port() function that gets called via a Discord command is the high-level module here and should only answer two questions:

• What needs to be done? The port needs to be changed.

• Why should it be done? Because the user requested it.

However, in this case, it also has to know details about "How is it done?" aka, it has to know about the concrete implementation of the database and therefore is strongly coupled with that specific implementation. If I wanted to change my database implementation, I would have to change the change_port command too.

The solution is quite simple: I could just use a DTO (Data Transfer Object) that the 'change_port' command generates and the 'update_server_value' method accepts; this way, neither rely on details and both are now readily switchable.

Here is a quick example:

# Data Transfer Object
@dataclass
class ServerUpdateRequest:
    server_id: int
    port: int

# Command without DIP violation
@commands.command(aliases=["chport"])
@commands.check(Checks.admin_check)
async def change_port(self, ctx, port:int):
    try:
        server_update_request = ServerUpdateRequest(server_id=ctx.guild.id, port=int(port))
        self.data_store.update_server_value(server_update_request)
        await ctx.send(f"I changed your port to {port}!")
    except ValueError:
        await ctx.send(f"{port} is not a number!")

Now it doesn't matter anymore how the 'update_server_value' method is implemented on the data access level as long as it takes the DTO as argument.

One more thing:
It's worth noting that I also replaced the coupling caused by explicitly using the MongoData class inside the command. When the bot instance is initialized, dependency injection should be used for the concrete database implementation.

Here is a very basic example of that:

class CustomBot(commands.Bot):
    def __init__(self, *args, database: IDatabase, **kwargs):
        super().__init__(*args, **kwargs)
        self.database = database

async def main():
    # Create Database instance
    db_instance = MongoDB(uri="mongodb_uri", database_name="database_name")

    # Initialize the bot with the database instance
    bot = CustomBot(command_prefix='!', database=db_instance)

    # Start the bot
    await bot.start(os.getenv('DISCORD_TOKEN'))

# Run the bot
asyncio.run(main())

YAGNI: The Art of Minimalism in Feature Implementation

Unlike SOLID, YAGNI(You Aren't Gonna Need It) is a basic principle that states, "Don't implement functionality until it's necessary."

This approach aims to avoid over-engineering and kEnhancing Code Quality with Object-Oriented Design and Minimalist Development Approacheseep the software development process focused on current requirements. YAGNI is violated when developers anticipate the requirement for a specific feature or functionality and implement it before it is actually required. My Bot, and especially the MongoData class, frequently break this guideline. There are methods like 'get_user_dcname_with_mcname' that allow you to query the database using a Discord name to return the minecraft name that is associated with it. In the 3 years that the bot is now running that command wasn't used once. That command has not been used once in the bot's three years of operation. So it's safe to presume that I can delete it without anyone noticing.

One key note:
YAGNI and SOLID may appear to conflict due to SOLID's emphasis on flexible, scalable, and extendable code structure, which may include, on the first glance, unnecessary interfaces and abstractions for clean and functional code.
However, the benefits of manageable, adaptable, and scalable code much exceed the tiny violation of YAGNI by abstractions and other systems that are not essential for the software to function.
When properly used, the principles are complementary rather than mutually exclusive. Keep the following in mind:

• SOLID principles ensure that your code is well-designed, manageable, and adaptable to the unavoidable changes that occur throughout software development.

• YAGNI assures that you don't waste time making modifications that aren't necessary.