Bad system design -- Overspecification of Data.

The key principles behind Architecture and technical drawing rules can and do apply to computer science and programing. One key item is "Over-specification". It is one of the main items that I find in;

• Poorly written or adhoc designed software
• In software that has been in long term maintenance without purging rewrites
• or in software produced by coders under constant(and often pointless) deadlines.

Here is what it is and why its evil.

Consider a Line with a point on it. Here are 2 classes defining it.

struct Good
{
  int length;
  int pointFromLeft;
};

struct Bad
{
  int length;
  int pointFromLeft;
  int pointFromRight;
};

So why is the Bad struct a no-no. Simple!. There is an implicate relationship between the data entries. "length = pointFromLeft + pointFromRight". If something updates the length and forgets to update one of the pointForm entries, then you end up with the data in an invalid state. This will later cause computation errors and finding the cause of the incorrect data is often quite difficult, because the code with will often look legitimate..

Furthermore the problem often isn't so simple to see/fix either. Consider this set of classes defining the same thing.

class GoodLineWithPoint
{
private:
  GoodPoint* point;
  int length;
};

class GoodPoint
}
private:
  int pointFromLeft;
}

class BadLineWithPoint
{
private:
  BadPoint* point;
  int length;
};

class BadPoint
}
private:
  int pointFromLeft;
  int pointFromRight;
}

In this case someone may have added the pointFromRight member so that they can "cache" the result. In reality the parent class GoodLineWithPoint should be the one that is dealt with however a function call might be passing GoodPoint to save the additional pointer deference. And once this is established in an API or lib it becomes quite difficult to fix cleanly.

Factory with registerable and configurable units..

One of the most common patterns used in software a factory. The ideal factory has sub units that can register with the main factory using one or more keywords.

The holy grail implementation is the self-registering factory, where the mere definition of the class causes it to register with the Factory. This is not really not really possible in C++ due to the limited RTT typing and introspection of the language. However you can do some 1 liner tricks to get the item to install at startup, such as static/local variables, or an instance function.

Below is a Factory that utilizes its registration to store the keyword, Type and some construction parameters for a factory. Thus allowing repeated of the same type with customized default parameters.

A brief class overview is

Class/Type	Description
Key	The description/tag/id for the factory
RegItemBase	The Factory Registration generic entry
Base	The Factory output class
RegFactory	The Factory
RegItem	An item to register with the factory, without any parameters
RegItemParam	An item to register with the factory, without a single parameter
A	The First demo class
B	The Second demo class

#include <iostream>
#include <map>

using namespace std;

/*****************************************************
 *          Factory with registering methods
 *****************************************************/

typedef string Key;

class Base;

class RegItemBase
{
public:
    RegItemBase(Key _key) :
        key(_key)
    {}

    virtual Base* construct()   const = 0;
    virtual void      print()   const = 0;
    virtual const Key keyword() const { return key; }

private:
    Key key;
};
typedef map<Key,RegItemBase*> KeyRegItemMap;

class Base
{
public:
    virtual const RegItemBase* getReg() const = 0;
    virtual const Key keyword() const { return getReg()->keyword(); }
    virtual void      print()   const { cout << keyword() << endl; }
    virtual void      bark()    const { cout << "...." << endl; }
};


class RegFactory
{
public:
    static RegFactory* getInstance() 
    {
        static RegFactory* instance = new RegFactory();
        return instance;
    }

    void add(RegItemBase* item) 
    {
        items[item->keyword()] = item;
    }
    
    Base* construct(Key key) 
    {
        KeyRegItemMap::const_iterator bit = items.find(key);
        if(bit != items.end())
            return bit->second->construct();
        return NULL;
    }
        
    void listAll() 
    {
        cout << "Registered items:"  << endl;

        KeyRegItemMap::const_iterator bit;
        for(bit = items.begin();
            bit != items.end();
            bit++)
        {
            cout << " " << bit->second->keyword() << endl;
        }

        cout << "Registered items DONE"  << endl;
    }

protected:
    RegFactory() {}

private:        
    KeyRegItemMap items;
};


template <class TempType>
class RegItem : public RegItemBase
{
public:
    typedef TempType Type;

    RegItem(const Key _key) :
        RegItemBase(_key)
    {
        cout << "Registering key: " << _key << endl;
        RegFactory::getInstance()->add(this);
    }

    virtual Base* construct()   const { return new Type(); }
    virtual void print()        const { cout << keyword() << endl; }
};

template <class TempType>
class RegItemParam : public RegItemBase
{
public:
    typedef TempType Type;

    RegItemParam(const Key _key, string _param) :
        RegItemBase(_key),
        param(_param)
    {
        cout << "Registering key: " << _key << endl;
        RegFactory::getInstance()->add(this);
    }

    virtual Base* construct()   const { return new Type(param); }
    virtual void print()        const { cout << keyword() << endl; }
private:
    string param;
};

/*****************************************************
 *             Specific Classes to register
 *****************************************************/

class A : public Base
{
public:
    static RegItemBase* reg()
    {
        static RegItemBase* regItem = (RegItemBase*)new RegItem<A>("A");
        return regItem;
    }
    virtual RegItemBase* getReg() const { return A::reg(); }

    virtual void      bark()    const { cout << "ruff" << endl; }
protected:
    friend class RegItem<A>;
    A() {}
};

class B: public Base
{
public:
    static RegItemBase* reg()
    {
        static RegItemBase* regItem  = (RegItemBase*)new RegItemParam<B>("B", "red");
        static RegItemBase* regItem2 = (RegItemBase*)new RegItemParam<B>("C", "green");
        return regItem;
    }
    virtual RegItemBase* getReg() const { return B::reg(); }

    virtual void      bark()    const { cout << "woof " << param << endl; }
protected:
    friend class RegItemParam<B>;
    B(string _param) : param(_param) {}

    string param;
};

int main()
{
    //setup regs...
    A::reg();
    B::reg();

    //list types..
    RegFactory::getInstance()->listAll();

    Base* a = RegFactory::getInstance()->construct("A");
    a->print();
    a->bark();

    Base* b = RegFactory::getInstance()->construct("B");
    b->print();
    b->bark();

    Base* c = RegFactory::getInstance()->construct("C");
    c->print();
    c->bark();
}

http://accu.org/index.php/journals/597

Object Anti-patterns

Object design patterns are great but there are many pit falls from over engineering a system or creating designs that become menaces, They have become know as the Anti-patterns.

Im currently trying to explain why a class that lists its objective as "This can either be seconds-from midnight. Or a time_t with a date." is a very bad idea.

I dont understand why people dont get it. Its basically a "Staralised schema" IE one object trying to be both things at the same time... and violates the "Single Responsibility principle"

UPDATE:
I have tried exploring the issues that occur with the implementation of an operator+ for this dual purpose class with them but they seem tied to the idea that what is in disk MUST be represented as is in memory.

I have also explain that when the class is viewed in isolate it loses the format information related to the time. This is because the Time classes main use is as an aggregate data-member and the aggregator classes type gives an implicit indication of what the Aggregatees time format is... I have even shown them code that proves the point and exposes many of the problems. To all this effort they have responded with "well maybe you should write/use a whole new time class for that"..

The main reason why I am fighting this so hard boils down the fact that these guys control QA in the system, and unless I can change there minds the problem will remain and other coders will continue to created hacks that avoid issues. These hacks then have a knock on effect(violation of Encapsulation principle) and become maintenance nightmares later...

Refer:
http://www.worldlingo.com/ma/enwiki/en/Anti-pattern

http://en.wikipedia.org/wiki/Single_responsibility_principle