Category: Software Architecture

Categories
Mobile Apps Software Architecture Software Development

HyperTrace 1.0 – My First Android Development Experience

This week, my company Danger Interactive LLC launched its first app, one for which all of the programming and art was done by myself, so in a sense, I launched my first app.

Learn more about that here, by the way: https://dangerzonegames.com/apps/hypertrace/

Experience

My preexisting experience with android development was very limited prior to developing this app. I had followed a tutorial one time over five years prior, before I even understood programming well. On another occasion, I planned to build an app for a friend, but once again, this was before my understanding of programming was sound enough to create even web apps. Thus, going into this project, I understood: basic Java programming concepts and how to launch Android Studio (although back when I did tutorials, there was no such program, it was all addons for Eclipse).

I spent approximately 2 months learning the Android API and building this app, which I think is a lot longer than it would take to duplicate it if I were to do it again, now that I know the ins and outs of the Android API and the Java programming language. Android is an excellent and easy to use API, and I am very happy I finally got around to learning it.

My Perspective on Android UI Reusability

In Android, the UI is composed of Activities, which is the main container for all other content. Usually only one activity is visible at a time, although there are ways to display partly transparent activities, but that’s not relevant for this discussion. Inside of these Activities, you have “Views” and “Fragments”. Everything that displays something to the screen is a View, and Fragments are a special kind.

Fragments, at first sight, seem to be an excellent way to create reusable chunks of Views. This is patently false. This is, in fact, what I suspect will be the primary mistake of new Android developers with regard to creating UI layouts.

There are a million reasons for this, but to make a long story short, Fragments are not meant to be a reusable chunk of views, in abstract. They are a special case. Fragments are not direct children of the Activity, and therefore they have an entirely different lifecycle. They are handled by the fragment manager. This separate lifecycle means that you can do really neat things like moving a fragment from Activity to Activity or changing the layout around.

However, if that’s not what you’re doing, then it is going to be a huge pain, because you cannot treat Fragments like regular views. All transactions must happen through the fragment manager. As you might imagine, this could also add additional overhead. So essentially, unless you need this special functionality, there’s a much better choice:

public class ReusableChunkOfLayoutView extends FrameLayout

This is what you should do. You will create a subclass of FrameLayout, which is your reusable chunk of layout, and then you’ll inflate a layout.xml file from that. It’s simple and straightforward, it offers the same ability to create XML layouts and build them easily, and it inherits all of the pre-built onLayout and onDraw code from FrameLayout, which is the rough equivalent of a “div” tag in HTML.

Reactive Code is Neat

I wrote the entire app using “Reactive” API design. What that essentially means is that each component that embodies some kind of state, network, or IO (especially network and IO actually) is capable of communicating with dependent objects. The means that I used to facilitate this communication channel was the Observer pattern.

public interface ObserverInterface {}

ObserverInterface was actually just an empty interface. It’s only there as a syntactic sugar to make the intent of a particular class clear to the programmer.

… public static class Observer implements ObserverInterface

Each class that needs to communicate with other classes creates a static inner class that implements this empty interface

… private SingleObservable<Observer> m_observable = new SingleObservable<>();

Then the class creates an instance of either SingleObservable or MultiObservable, through which all of the notifications will be passed. It is a generic class because ObserverInterface doesn’t offer any methods, so we need to have no erasure in order to be able to call the methods that are added to the observer. This is a good example of functionality via composition instead of inheritance.

public interface ObserverAction<T extends ObserverInterface>

The ObserverAction class represents a “notification” essentially. Once again, it’s generic to avoid the erasure issues. The interface contains a single public method called “run” that takes a single argument of type “T” (the observer). This observer argument is used to be able to call methods on the observer (the real notification channel). The SingleObservable and MultiObservable classes receive these objects and apply them to each relevant observer.

public class SingleObservable<T extends ObserverInterface>

The SingleObservable class was a relatively straightforward one. It just needs to have a single private member that holds one element of type T (the observer), and an ArrayList of ObserverAction objects. The ObserverAction list is there because of what I termed “final actions” which represent actions that should be run on every observer added after the action occurs. This is good for such things as observers with an “onComplete” method, where you can be certain that it will only be called once, and therefore the reasonable assumption is that later observers might want to be informed of this completion.

This class includes a “setObserver” method, and then a “run” and a “runFinal” method, which are provided the ObserverActions to be run once, and run forever, respectively.

public class MultiObservable<T extends ObserverInterface>

The MultiObservable class was a little more complex. There are thread-safety issues that need to be considered in this case. This observable needs two ArrayLists of objects of type T, one of which is the observers, and the other is the pending observers. Pending observers is the holding patter for observers that try to get added while a notification is being processed. Thus, there is a boolean flag that represents whether or not the observers are being notified, and if so, “addObserver” will place the observer in pending, and then the running “runAll” or “runAllFinal” methods will unset the flag and move all of the objects to the observers list and clear the pending observers.

This class includes different methods. There is an “addObserver” method, and then “runAll” and “runAllFinal” which do the equivalent of “run” and “runFinal” in SingleObserver, but they iterate through every object in the observers list and runs the ObserverAction on the current observer. As mentioned, on completion they move anything from pending to the regular observers list.

The Bad… The Ugly…

There are exactly two things that I hated to use when working with Android:

Scrolling

To do scrolling, there are three relevant views. There’s ScrollView, HorizontalScrollView, and NestedScrollView. What do you need to know about them? ScrollView is deprecated and useless, just use NestedScrollView instead, HorizontalScrollView is the only scroll view capable of horizontal scrolling, and there is no way to create a single view that scrolls on both axes. You must nest a HorizontalScrollView inside of a NestedScrollView (or vice versa).

Maximum Height/Width

There is no such thing. I have actually not found a good way to do this either. I guess I’ll update anybody on a solution if I ever find one. This is a tragic issue because max-height and max-width are extremely useful when creating responsive layouts. Obviously this is less of a concern on Android, since the screen and window size isn’t expected to change much.

Speaking of responsive layouts, your best bet for creating anything that can be termed “responsive” will likely involve a ConstraintLayout or two. Learn to use them. You won’t regret it.

Categories
Game Development Software Architecture Software Development

The Scourge of Global Mutable State

I’m not a perfect programmer, but I like to think that I’m getting better every day. I’ll admit that one of the things that I’ve had the most difficult time adapting to was what I perceived as other programmers being somewhat pedantic about avoiding what they called “global state” or sometimes more specifically “global mutable state.”

In the past, I held onto some conviction that while this is something to be avoided where possible, there are actually cases where it is appropriate. Recently I’ve decided to not only better conform to this well-known best-practice, but also to understand why it is that this is an issue, and what can be done to achieve what I, in the past, mistakenly perceived as the reason that global mutable state made sense, without violating this fundamental rule.

To learn something, you must learn the rules, but to become an expert, you must learn when to break these same rules. Programming is no different. However, over several years of programming I’ve come to find that the “when” of “when to break the rules” is an exception, not the rule. If you’re breaking the rule more often than not, then you’re probably misunderstanding something or being lazy. It pays to heed the words of the experts superior to you in experience even when you don’t fully understand why, because eventually, with more experience, you will understand why, and if you failed to heed their advice, you’ll find yourself chastising your past self for it.

Let me start by explaining why it is that I’ve so often resisted this best practice and gone ahead with global mutable state anyway. When writing larger code bases, which most projects inevitably aspire to be, you will find that some of your code will be, as a matter of necessity, used by a large proportion of the code base. Using dependency injection as your only distribution technique, as an example, might drastically increase the verbosity of your code. Who wants to pass an instance a “Log” class to every single instance of a “GameState” object? I’ll admit that it’s not really THAT much extra work, but it has a distinct feeling of not being “DRY,” since I’m essentially repeating myself on every object instantiation. There are some subsystems which, by nature, the entire system will need to access, and would be better off being organized.

A pet peeve not only of myself, but of many people, is that some people seem to think that using the singleton pattern is a solution to this problem. Singletons are global state, and moreover, it appears to me that by now, the singleton is obsolete. Any problem solvable with a singleton can be solved with a static class with mutable private fields, and frankly these static classes don’t lie about their true nature. But both are global state, and both ought to be avoided anyway. But if you’re going to ignore the advice anyway, you might as well just use the static class instead, so users can recognize the warning signs of nondeterministic behavior before it’s too late.

I’ve learned, however, that all of my arguments came from a lack of understanding of the actual problem, and the actual solutions.

  • Global mutable state generally excludes communication channels that allow decoupled non-global objects to communicate (such as Observers or Service Locators). This is a significant solution to this problem.
  • Global mutable state implies that the global items in question contain actual values that change between subsequent calls. Global classes are still perfectly valid for resolving the problem of subsystems that need to be accessed from across the system as whole, so long as they incorporate the solutions from the previous bullet.

I’ll document one solution that I solved with my TimberWolf game engine.

In the past, I’ve had a static class named Log, which was declared as follows:

#ifndef H_CLASS_LOG
#define H_CLASS_LOG

#include <iostream>
#include <string>
#include <iomanip>
#include <sstream>
#include <fstream>
#include <ctime>
#include <mutex>
#include <exception>
#include <cstdlib>
#include <atomic>
#include "../../enum/LogLevel/LogLevel.hpp"

class Log {

public:

    Log () = delete; // static only
    ~Log () = delete;

    Log (Log&&) = delete;
    Log& operator = (Log&&) = delete;

    Log (const Log&) = delete;
    Log& operator = (const Log&) = delete;

    static bool cliOutputEnabled ();
    static void enableCliOutput ();
    static void disableCliOutput ();

    static LogLevel getCliFilterLevel ();
    static void setCliFilterLevel (LogLevel);

    static bool fileOpen ();
    static bool openFile (const std::string&);
    static void closeFile ();

    static bool fileOutputEnabled ();
    static void enableFileOutput ();
    static void disableFileOutput ();

    static LogLevel getFileFilterLevel ();
    static void setFileFilterLevel (LogLevel);

    template<typename ...T>
    static void log (LogLevel messageType, T&&... message) {

        std::string out;

        if (
            (m_cliOutputEnabled || m_fileOutputEnabled) &&
            (
                messageType == LogLevel::UNDEFINED ||
                messageType >= m_cliFilterLevel ||
                messageType >= m_fileFilterLevel
            )
        ) {
            out = Log::formatMessage(messageType, Log::concatMessage(std::forward<T>(message)...));
        } else {
            return;
        }

        if (m_cliOutputEnabled) {
            if (messageType == LogLevel::UNDEFINED || messageType >= m_cliFilterLevel) {

                if (messageType == LogLevel::ERROR) {
                    std::unique_lock<std::mutex> lock_stderr(Log::mutex_stderr);
                    std::cerr << out << std::endl;
                } else {
                    std::unique_lock<std::mutex> lock_stdout(Log::mutex_stdout);
                    std::cout << out << std::endl;
                }

            }
        }

        if (m_fileOutputEnabled && fileOpen()) {
            if (messageType == LogLevel::UNDEFINED || messageType >= m_fileFilterLevel) {

                std::unique_lock<std::mutex> lock_file(Log::mutex_file);
                m_file << out << std::endl;

            }
        }

    }

    template<typename ...T>
    static void verbose (T&&... message) {
        Log::log(LogLevel::VERBOSE, std::forward<T>(message)...);
    }

    template<typename ...T>
    static void notice (T&&... message) {
        Log::log(LogLevel::NOTICE, std::forward<T>(message)...);
    }

    template<typename ...T>
    static void warning (T&&... message) {
        Log::log(LogLevel::WARNING, std::forward<T>(message)...);
    }

    template<typename ...T>
    static void error (T&&... message) {
        Log::log(LogLevel::ERROR, std::forward<T>(message)...);
    }

    static void bindUnhandledException ();
    static void unhandledException ();

private:

    template<typename ...T>
    static std::string concatMessage (T&&... message) {

        std::ostringstream oss;
        (oss << ... << std::forward<T>(message));
        return oss.str();

    }

    static std::string formatMessage (LogLevel, const std::string&);

    static std::atomic<bool> m_cliOutputEnabled;
    static std::atomic<bool> m_fileOutputEnabled;

    static std::atomic<LogLevel> m_cliFilterLevel;
    static std::atomic<LogLevel> m_fileFilterLevel;

    static std::ofstream m_file;

    static std::mutex mutex_stdout;
    static std::mutex mutex_stderr;
    static std::mutex mutex_file;

};

#endif

As you can see, everything is static, and the class cannot be instantiated. This makes sense, because the Log class is something that needs to be accessed from all over the code base, and we generally want all log messages to funnel through a single location so none of them accidentally go astray before they are sent or stored. This was all fine and dandy when it was originally written, because it only had one purpose then, and that was to write error messages to std::cout or std::cerr. It interacted with a global I/O, but it didn’t store any kind of state.

After a while, though, I decided that a few features would be nice to have in the logging subsystem. First of all, I wanted to be able to filter log messages by their LogLevel enum. This was simple enough. Add a static field to store the minimum log level, and then compare the log level of each message before writing it to I/O. But now the log class contains the global state for the current log level. Anybody could change that, and it might make it hard to debug stuff because its behavior becomes nondeterministic. The behavior of the Log class depends on the order of modifications. If some code somewhere deep in the code base calls Log::setFilterLevel(LogLevel::ERROR) because the developer was getting inundated with verbose calls and wanted to temporarily silence them, and then forgot to remove it, all LogLevel::WARNING messages sent after that piece of code gets invoked will fail to display. The developer might mistakenly think that no errors are occurring. And there’s no way to receive those messages elsewhere unless the filter level is dropped again.

This hasn’t actually been a problem for me yet, but I was slightly aware of the possibility, but I hadn’t yet done anything to resolve it. I added additional features, such as allowing Log to write to a log file, and allowing a separate filter level for the log file. I noticed some additional potential problems after adding these features, however. First of all, I can only write to a single log file. What if I decide later on that I want to write only warnings and errors to one file, and all messages to another file. Or what if I want a console window in my game somewhere to display the error messages, without requiring the end-user to look at their terminal or open up a log file?

The Log subsystem is a problem that can be solved by the observer pattern. I can leave the API for sending log messages alone, because it implements everything the observables need (but I am going to add a context string that can be used for additional filtering, in case we want the log messages pertaining only to the sound system to go somewhere, for example). There will need to be a separate API for RECEIVING the log messages, however. One of these will be used to receive messages and route them to the command line. Another one will be used to put messages into a file. But more can be created and removed as needed.

If those changes are implemented, The Log class, which is the global portion of the subsystem, will no longer contain any global state at all (the vectors of observers are not technically “state” in this context, as they are not going to have any effect on one another, the log subsystem, or the game as a whole). Any code in the code base can send messages to it, but the output, which is the real tangible effect of the log subsystem, will only be controlled by non-global objects. Code far across the code base will not be able to create nondeterministic behavior in the logs of an unrelated system.

This is, for all intents and purposes, the best of both worlds. I can expose a global API without needing to define behavior globally. I can create a log subsystem that doesn’t require dependency injection, but also doesn’t expose any state to the global scope, while still supporting stateful behavior at one end of the pipeline.

With that explanation out of the way now, allow me to show you the finished solution. Note that between the old version and this current one, the namespace “tw” was added to everything.

Here’s the new Log class declaration:

#ifndef H_CLASS_LOG
#define H_CLASS_LOG

#include <iostream>
#include <string>
#include <vector>
#include <memory>
#include <sstream>
#include <mutex>
#include <functional>
#include "../LogObserver/LogObserver.hpp"
#include "../../enum/LogLevel/LogLevel.hpp"

namespace tw {
class Log {

public:

    Log () = delete; // static only
    ~Log () = delete;

    Log (Log&&) = delete;
    Log& operator = (Log&&) = delete;

    Log (const Log&) = delete;
    Log& operator = (const Log&) = delete;

    template <typename ...T>
    static void log (LogLevel messageType, const std::string& context, T&&... message) {

        std::unique_lock<std::mutex> lock(m_mutex);

        for (unsigned int i = 0; i < m_observers.size(); ++i) {

            m_observers[i]->notify(messageType, context, concatMessage(std::forward<T>(message)...));

        }

    }

    template <typename ...T>
    static void verbose (const std::string& context, T&&... message) {
        Log::log(LogLevel::VERBOSE, context, std::forward<T>(message)...);
    }

    template <typename ...T>
    static void notice (const std::string& context, T&&... message) {
        Log::log(LogLevel::NOTICE, context, std::forward<T>(message)...);
    }

    template <typename ...T>
    static void warning (const std::string& context, T&&... message) {
        Log::log(LogLevel::WARNING, context, std::forward<T>(message)...);
    }

    template <typename ...T>
    static void error (const std::string& context, T&&... message) {
        Log::log(LogLevel::ERROR, context, std::forward<T>(message)...);
    }

    static void bindUnhandledException ();

    template <typename T, typename ...Targ>
    static LogObserver* makeObserver (Targ&&... args) {

        auto observer = std::make_unique<T>(std::forward<Targ>(args)...);
        auto observerPtr = observer.get();

        m_observers.push_back(std::move(observer));

        return observerPtr;

    }
    static void registerObserver (std::unique_ptr<LogObserver>&&);
    static void registerObserver (LogObserver*);

private:

    template <typename ...T>
    static std::string concatMessage (T&&... message) {

        std::ostringstream oss;
        (oss << ... << std::forward<T>(message));
        return oss.str();

    }

    static std::mutex m_mutex;
    static std::vector<std::unique_ptr<LogObserver>> m_observers;

};
}

#endif

As you can see, the Log class is now nothing more than a shell, a notification system for a collection of observers (and also a system for creating and managing said observers). It presents a new class known as LogObserver, which is a virtual class.

#ifndef H_CLASS_LOGOBSERVER
#define H_CLASS_LOGOBSERVER

#include <string>
#include <set>
#include "../../enum/LogLevel/LogLevel.hpp"
namespace tw{ class Log; }

namespace tw {
class LogObserver {

// WARNING: Never call Log::* functions from inside subclasses of this.
// It will cause infinite recursion and you will be sad.

public:

    static inline constexpr unsigned int ALLOW_UNDEFINED = 0b00001;
    static inline constexpr unsigned int ALLOW_VERBOSE =   0b00010;
    static inline constexpr unsigned int ALLOW_NOTICE =    0b00100;
    static inline constexpr unsigned int ALLOW_WARNING =   0b01000;
    static inline constexpr unsigned int ALLOW_ERROR =     0b10000;

    LogObserver () = default;
    explicit LogObserver (unsigned int);
    explicit LogObserver (const std::set<std::string>&);
    explicit LogObserver (const std::string&...);
    LogObserver (unsigned int, const std::set<std::string>&);
    LogObserver (unsigned int, const std::string&...);
    virtual ~LogObserver () = default;

    LogObserver (LogObserver&&) = default;
    LogObserver& operator = (LogObserver&&) = default;

    LogObserver (const LogObserver&) = default;
    LogObserver& operator = (const LogObserver&) = default;

    void notify (LogLevel, const std::string&, const std::string&);

    bool undefinedAllowed () const;
    void allowUndefined ();
    void blockUndefined ();
    bool verboseAllowed () const;
    void allowVerbose ();
    void blockVerbose ();
    bool noticeAllowed () const;
    void allowNotice ();
    void blockNotice ();
    bool warningAllowed () const;
    void allowWarning ();
    void blockWarning ();
    bool errorAllowed () const;
    void allowError ();
    void blockError ();

    bool contextAllowed (const std::string&...) const;
    void allowContext (const std::string&...);
    void blockContext (const std::string&...);
    bool allContextAllowed () const;
    void allowAllContext ();
    void restrictContext ();

protected:

    virtual void notifyCallback (LogLevel, const std::string&, const std::string&) = 0;

    unsigned int m_allowedLevelFlags {
        ALLOW_UNDEFINED |
        ALLOW_VERBOSE |
        ALLOW_NOTICE |
        ALLOW_WARNING |
        ALLOW_ERROR
    };

    std::set<std::string> m_allowedContexts;
    bool m_allContextsAllowed {true};

};
}

#endif

LogObserver classes contain non-overrideable fields and methods for storing and manipulating data about the log levels and contexts that it’s interested in. The notify method is markedly non-virtual because it implements the code for determining if the LogObserver even cares about the message that’s being sent to notify. As such, notify is the method that is initially invoked whenever a log message is being processed. If all of the checks pass, then it is finally forwarded on to the protected virtual method notifyCallback, which is to be overridden to the tastes and needs of the implementor.

Let’s look at some examples of implementations, the few that I’ve already made, perhaps all that I’ll ever need:

#ifndef H_CLASS_LOGOBSERVER
#define H_CLASS_LOGOBSERVER

#include <string>
#include <set>
#include "../../enum/LogLevel/LogLevel.hpp"
namespace tw{ class Log; }

namespace tw {
class LogObserver {

// WARNING: Never call Log::* functions from inside subclasses of this.
// It will cause infinite recursion and you will be sad.

public:

    static inline constexpr unsigned int ALLOW_UNDEFINED = 0b00001;
    static inline constexpr unsigned int ALLOW_VERBOSE =   0b00010;
    static inline constexpr unsigned int ALLOW_NOTICE =    0b00100;
    static inline constexpr unsigned int ALLOW_WARNING =   0b01000;
    static inline constexpr unsigned int ALLOW_ERROR =     0b10000;

    LogObserver () = default;
    explicit LogObserver (unsigned int);
    explicit LogObserver (const std::set<std::string>&);
    explicit LogObserver (const std::string&...);
    LogObserver (unsigned int, const std::set<std::string>&);
    LogObserver (unsigned int, const std::string&...);
    virtual ~LogObserver () = default;

    LogObserver (LogObserver&&) = default;
    LogObserver& operator = (LogObserver&&) = default;

    LogObserver (const LogObserver&) = default;
    LogObserver& operator = (const LogObserver&) = default;

    void notify (LogLevel, const std::string&, const std::string&);

    bool undefinedAllowed () const;
    void allowUndefined ();
    void blockUndefined ();
    bool verboseAllowed () const;
    void allowVerbose ();
    void blockVerbose ();
    bool noticeAllowed () const;
    void allowNotice ();
    void blockNotice ();
    bool warningAllowed () const;
    void allowWarning ();
    void blockWarning ();
    bool errorAllowed () const;
    void allowError ();
    void blockError ();

    bool contextAllowed (const std::string&...) const;
    void allowContext (const std::string&...);
    void blockContext (const std::string&...);
    bool allContextAllowed () const;
    void allowAllContext ();
    void restrictContext ();

protected:

    virtual void notifyCallback (LogLevel, const std::string&, const std::string&) = 0;

    unsigned int m_allowedLevelFlags {
        ALLOW_UNDEFINED |
        ALLOW_VERBOSE |
        ALLOW_NOTICE |
        ALLOW_WARNING |
        ALLOW_ERROR
    };

    std::set<std::string> m_allowedContexts;
    bool m_allContextsAllowed {true};

};
}

#endif

ConsoleLogObserver is a specialization that sends the messages to the console.

#ifndef H_CLASS_FILELOGOBSERVER
#define H_CLASS_FILELOGOBSERVER

#include <string>
#include <set>
#include <ctime>
#include <iomanip>
#include "../LogObserver/LogObserver.hpp"
#include "../File/File.hpp"

namespace tw {
class FileLogObserver : public LogObserver {

public:

    FileLogObserver () = default;
    explicit FileLogObserver (const std::string&);
    FileLogObserver (const std::string&, unsigned int);
    FileLogObserver (const std::string&, const std::set<std::string>&);
    FileLogObserver (const std::string&, const std::string&...);
    FileLogObserver (const std::string&, unsigned int, const std::set<std::string>&);
    FileLogObserver (const std::string&, unsigned int, const std::string&...);
    ~FileLogObserver () override = default;

    FileLogObserver (FileLogObserver&&) = default;
    FileLogObserver& operator = (FileLogObserver&&) = default;

    FileLogObserver (const FileLogObserver&) = default;
    FileLogObserver& operator = (const FileLogObserver&) = default;

    std::string getFilePath () const;
    bool setFilePath (const std::string&);

protected:

    virtual void notifyCallback (LogLevel, const std::string&, const std::string&) override;

    virtual std::string formatMessage (LogLevel, const std::string&, const std::string&);

    File m_file {"./log.txt", File::ENABLE_WRITE | File::ENABLE_APPEND};

};
}

#endif

FileLogObserver is a specialization that stores a file handle and writes log messages to that file.

And finally, FunctionLogObserver is a specialization that stores and manages a collection of function objects that receive the message (so in a way, it can do anything).

#ifndef H_CLASS_FUNCTIONLOGOBSERVER
#define H_CLASS_FUNCTIONLOGOBSERVER

#include <string>
#include <set>
#include <vector>
#include <functional>
#include <mutex>
#include "../LogObserver/LogObserver.hpp"

namespace tw {
class FunctionLogObserver : public LogObserver {

public:

    typedef std::function<void(LogLevel, const std::string&, const std::string&)> Callback;

    FunctionLogObserver () = default;
    explicit FunctionLogObserver (const std::vector<Callback>&);
    explicit FunctionLogObserver (const Callback&...);
    FunctionLogObserver (const std::vector<Callback>&, unsigned int);
    FunctionLogObserver (const Callback&, unsigned int);
    FunctionLogObserver (const std::vector<Callback>&, const std::set<std::string>&);
    FunctionLogObserver (const Callback&, const std::set<std::string>&);
    FunctionLogObserver (const std::vector<Callback>&, const std::string&...);
    FunctionLogObserver (const Callback&, const std::string&...);
    FunctionLogObserver (const std::vector<Callback>&, unsigned int, const std::set<std::string>&);
    FunctionLogObserver (const Callback&, unsigned int, const std::set<std::string>&);
    FunctionLogObserver (const std::vector<Callback>&, unsigned int, const std::string&...);
    FunctionLogObserver (const Callback&, unsigned int, const std::string&...);
    ~FunctionLogObserver () = default;

    FunctionLogObserver (FunctionLogObserver&&) = default;
    FunctionLogObserver& operator = (FunctionLogObserver&&) = default;

    FunctionLogObserver (const FunctionLogObserver&) = default;
    FunctionLogObserver& operator = (const FunctionLogObserver&) = default;

    const std::vector<Callback>& getCallbacks () const;
    void setCallbacks (const std::vector<Callback>&);
    void addCallback (const Callback&...);
    void clearCallbacks ();

protected:

    virtual void notifyCallback (LogLevel, const std::string&, const std::string&);

    std::mutex m_mutex;
    std::vector<Callback> m_callbacks;

};
}

#endif

If you’d like to read the implementation .cpp files, those are available in the git repo. They didn’t seem to be necessary to illustrate the design pattern. Also, this example code isn’t perfect, and I’m sure in short order I’ll find reason to modify it. At the time of writing, TimberWolf doesn’t have a stable API, so please don’t use this as documentation for it.

Categories
Software Architecture Software Development Web Development

Software Architecture and the YAGNI principle

I consider myself a developer with a good appreciation for a well-designed and carefully implemented architecture. I like to do things the “right” way whenever possible, instead of the quick way. I try my best to make everything I write modular, with minimal dependencies, and adaptable to any platform. Over the years I’ve been programming, however, I’ve learned that while that is a noble goal, there are certain occasions where the effort to conform to the aforementioned goal is not actually worth it. The key driving principle behind what I am referring to is the “YAGNI principle.”

YAGNI, meaning “You Aren’t Gonna Need It,” is the idea that you should never add a feature to your code base that you aren’t certain that you are going to need soon, or even eventually. I think many software developers who are relatively new to the craft, and who begin to learn about all of the architectural design patterns that can be used to keep your code base from turning into something that keeps you awake at night, sweating with fear, there is a tendency to begin developing a fear of cutting any corners ever. There is a general sense that making things more “smart” is always worth the effort, regardless of the cost. But YAGNI says that is not so. This was a principle that I unfortunately had to learn the hard way about 3 years ago.

I was working on an (unfortunately closed source) web-based asset management system software for my previous employer. Believe it or not, this was the first piece of professional software that I worked on (as in, the first that I was getting paid to make). I started work on it in mid 2014, and at that time I was making it with a fairly difficult-to-maintain procedural PHP code base, with no libraries or frameworks whatsoever. No functionality was being delegated to JavaScript, it was just going to be PHP doing all the work and sending a big chunk of HTML. In other words, it was an atrocious mountain of spaghetti at that point.

During that year, I worked on a lot of other projects, and began to learn quite a bit more about the PHP language, and above all, I decided that I was high time I learned how to use Object Oriented Programming to make my code a lot more maintainable, “DRY” if you will. So I dug into some tutorials and messed around with it, and got a general sense of how it worked. I also started learning about basic template systems, AJAX, and RESTful principles.

Some time in 2015, I decided to take all of this new-found knowledge back to the old asset management system code base, and try to apply it everywhere I could. And so I spent a large amount of time modernizing the code base and getting it up to par with what I thought was excellent design. And most of the ideas I had stuck around through the remaining years, more or less, so I know I wasn’t completely misguided.

There was one idea that I had, however, that was the biggest waste of time, and one of my absolute biggest regrets in the entire project. This is the purpose of this postmortem: to learn from this mistake that cost countless hours of valuable development time.

I decided that it would be cool to make this piece of software as portable as possible, so it was easy to install on any system my employer would ever use. I followed a lot of the patterns that I’d seen in other self-hosted web applications. Despite the fact that all of our apps only used MySQL as a database server, I wanted to make it possible to run it on MySQL, PostgreSQL, and SQLite. I also wanted to make this feature a library that I could use in other projects, and other people could use in theirs, so I did release that code under the MIT license on GitHub.

That idea, in itself, is not always bad. There are truly some occasions where doing such a thing would actually be valuable. They are all relational databases, so despite some syntactical and feature differences, they operate on the same core concepts. The way I decided to implement this cross-sql-platform support was by abstracting the entire concept of a relational database into PHP, and having that PHP act as a code generator writing SQL for prepared statements and submitting the relevant data. Essentially the idea was that instead of writing SQL queries, I would have database objects with methods allowing me to modify the structure and data in the database. The library would have to be pretty intelligent in figuring out how to safely and efficiently write a SQL query to perform the requested operation, so there were certainly security and performance risks associated with it.

I spent months of time adding each feature and testing them one by one as I started to integrate it with the existing code base. For all of the features that I actually integrated and tested, it actually worked quite well. The problem, though, was the incredible complexity of the system. SQL is implemented as a full programming language because of that complexity, and somehow I didn’t grasp that at the beginning of the project.

At some point, well into the project and not making the kind of traction that I wanted to make, I kept the library code on GitHub as before, but I scrapped integrating it into the asset management system, instead opting for another database abstraction design that I had learned and found to be much more reasonable for a project that is unlikely to ever be moved from the RDBMS that it started on (but is still flexible enough if it does). That approach was to create a single static class representing the database, with very basic abstraction methods as private, utilized by the public methods representing more high-level operations like inserting a new item, or editing an existing department. This means that all of the database-related code is in this single file, so it’s easy to change it whenever needed, but I can safely assume that any operation can be written specifically for MySQL. If the RDBMS needs to be changed one day, then the queries in this file would need to be rewritten, but the rest of the code base would not need to be touched, as the interface would remain entirely the same.

Most of the benefits of the abstraction library, at a fraction of the complexity. This design took far fewer lines of code to implement, and took a much shorter time. From the ground up, all features implemented in this static class, took less than half the time that I spent on a still-incomplete full database abstraction.

So the lesson to be learned here is what? Don’t implement a full database abstraction because it’s a waste of time? Absolutely not. Like I alluded to before, there are certainly some projects that would benefit from such a thing, and in those cases, you’ll just have to suck it up and trudge through thousands of lines and dozens of classes to make it work. But the question you should be asking before you go off on such a costly mission is an honest estimate of how often you expect the code base to switch its RDBMS. Maybe you’re marketing your software to a lot of varied users who may not want to install your database server of choice. In that case, you might want to consider it. But in my case, simply asking myself that question, the answer would have clearly been “probably never, maybe once in 5 to 10 years”. I should not have done it. Frankly, the cost of rewriting the queries in the static class MAYBE once would have been much less than the amount of time that I spent writing that database abstraction library.

So remember the YAGNI principle during the design phase of your project. Whenever you see something potentially complex getting added to the design, make sure to ask yourself, “Are you really gonna need it?” and if the answer isn’t a certain “yes,” then just remember that it’s okay to rewrite things later on when you have more information. Refactoring your code later just might be a lot cheaper and better than worrying about the minute details so early on. The finished product is rarely the same as the finished concept anyway, so be an adaptable programmer first, and write an adaptable code base later.