Applying SOLID principles

Question

I am quite new to the S.O.L.I.D. design principles. I understand their cause and benefits, but yet i fail to apply them to a smaller project which I want to refactor as a practical exercise to use the SOLID principles. I know there is no need to change an application that works perfectly, but I want to refactor it anyway so I gain design experience for future projects.

The application has the following task (actually a lot more than that but let's keep it simple): It has to read an XML file which contains Database Table/Column/View etc definitions and create an SQL file which can be used in order to create an ORACLE database schema.

(Note: Please refrain from discussing why I need it or why I don't use XSLT and so on, there are reasons, but they are off-topic.)

As a start, I chose to look only at Tables and Constraints. If you ignore columns, you could state it the following way:

A constraint is part of a table (or more precisely, part of a CREATE TABLE statement), and a constraint may also reference another table.

First, I will explain what the application looks like right now (not applying SOLID):

At the moment, the application has a "Table" class which contains a list of pointers to Constraints owned by the table, and a list of pointers to Constraints referencing this table. Whenever a connection gets established, the backwards connection will be established as well. The table has a createStatement() method which in turn calls the createStatement() function of each Constraint. Said method will itself use the connections to the owner table and referenced table in order to retrieve their names.

Obviously, this doesn't apply to SOLID at all. For example, there are circular dependencies, which bloated the code in terms of "add"/"remove" methods required and some large object destructors.

So there are a couple of questions:

1. Should I resolve the circular dependencies using Dependency Injection? If so, I suppose the Constraint should receive the owner (and optionally the referenced) table in its constructor. But how could I run over the list of constraints for a single table then?
2. If the Table class both stores the state of itself (e.g. table name, table comment etc) and the links to Constraints, are these one or two "responsibilities", thinking of the Single Responsibility Principle?
3. In case 2. is right, should I just create a new class in the logical business layer which manages the links? If so, 1. would obviously no longer be relevant.
4. Should the "createStatement" methods be part of the Table/Constraint classes or should I move them out as well? If so, where to? One Manager class per each data storage class (i.e. Table, Constraint, ...)? Or rather create a manager class per link (similar to 3.)?

Whenever I try to answer one of these questions I find myself running in circles somewhere.

The problem obviously gets a lot more complex if you include columns, indices and so on, but if you guys help me out with the simple Table/Constraint thing, I can maybe work out the rest on my own.

Answer 1

You may start from a different point of view to apply "Single Responsibility Principle" here. What you have shown to us is (more or less) only the data model of your application. SRP here means: make sure your data model is responsible only for keeping data - no less, no more.

So when you are going to read your XML file, create a data model from it and write SQL, what you should not do is implement anything into your Table class which is XML or SQL specific. Your want your data flow look like this:

[XML] -> ("Read XML") -> [Data model of DB definition] -> ("Write SQL") -> [SQL]

So the only place where XML specific code should be placed is a class named, for instance, Read_XML. The only place for SQL specific code should be a class like Write_SQL. Of course, maybe you are going to split those 2 tasks into more sub-tasks (and split your classes into multiple manager classes), but your "data model" should not take any responsibility from that layer. So don't add a createStatement to any of your data model classes, since this gives your data model responsibility for the SQL.

I don't see any problem when you are describing that a Table is responsible for holding all it's parts, (name, columns, comments, constraints ...), that is the idea behind a data model. But you described "Table" is also responsible for the memory management of some of its parts. That's a C++ specific issue, which you would not face so easily in languages like Java or C#. The C++ way of getting rid of those responsibility is using smart pointers, delegating ownership to a different layer (for example, the boost library or to your own "smart" pointer layer). But beware, your cyclic dependencies may "irritate" some smart pointer implementations.

Something more about SOLID: here is nice article.

explaining SOLID by a small example. Let's try to apply that to your case:

• you will need not only classes Read_XML and Write_SQL, but also a third class which manages the interaction of those 2 classes. Lets call it a ConversionManager.

• Applying DI principle could mean here: ConversionManager should not create instances of Read_XML and Write_SQL by itself. Instead, those objects can be injected through the constructor. And the constructor should have a signature like this

  ConversionManager(IDataModelReader reader, IDataModelWriter writer)

where IDataModelReader is an interface from which Read_XML inherits, and IDataModelWriter the same for Write_SQL. This makes a ConversionManager open for extensions (you very easily provide different readers or writers) without having to change it - so we have an example for the Open/Closed principle. Think about it what you will have to change when you want to support another database vendor -ideally, you don't have to change anything in your datamodel, just provide another SQL-Writer instead.

Answer 2

Well, you should apply the S of SOLID in this case.

A table holds all constraints defined on it. A constraint holds all tables it references. Plain and simple model.

What you stick into that, is the ability to perform inverse lookups, i.e. to figure out by which constraints some table is referenced.
So what you actually want is an indexing service. That is a completely different task and should therefore be carried out by a different object.

To break it down to a very simplified version:

class Table {
      void addConstraint(Constraint constraint) { ... }
      bool removeConstraint(Constraint constraint) { ... }
      Iterator<Constraint> getConstraints() { ... }
}
class Constraint {
      //actually I am not so sure these two should be exposed directly at all
      void addReference(Table to) { ... }
      bool removeReference(Table to) { ... }
      Iterator<Table> getReferencedTables() { ... }
}
class Database {
      void addTable(Table table) { ... }
      bool removeTable(Table table) { ... }
      Iterator<Table> getTables() { ... }
}
class Index {
      Iterator<Constraint> getConstraintsReferencing(Table target) { ... }
}

As for the implementation of the index, there's 3 ways to go:

• the getContraintsReferencing method could really just crawl the whole Database for Table instances and crawl their Constraints to get the result. Depending on how costly this is and how often you need it, it may be an option.
• it could also use a cache. If your database model can change once defined, you can maintain the cache by firing signals from the respective Table and Constraint instances, when they change. A slightly simpler solution would be to have the Index build up a "snapshot index" of the whole Database to work with, that you would then discard. That's of course only possible, if your application makes a big distinction between "modelling time" and "querying time". If it's rather likely to do those two at the same time, then this is not viable.
• Another option would be to use AOP to intercept the whole creation calls and maintain the index accordingly.

Answer 3

The cure for circular dependencies is to vow that you will never, ever create them. I find that coding test-first is a strong deterrent.

Anyway, circular dependencies can always be broken by introducing an abstract base class. This is typical for graph representations. Here the tables are nodes and the foreign key constraints are edges. So create an abstract Table class and an abstract Constraint class and maybe an abstract Column class. Then all the implementations can depend on the abstract classes. This may not be the best possible representation, but it is an improvement over mutually coupled classes.

But, as you suspect, the best solution to this problem may not require any tracking of object relationships. If you only want to translate XML to SQL, then you don't need an in-memory representation of the constraint graph. The constraint graph would be nice if you wanted to run graph algorithms, but you didn't mention that so I'll assume it's not a requirement. You just need a list of tables and a list of constraints and a visitor for each SQL dialect you want to support. Generate the tables, then generate the constraints external to the tables. Until the requirements changed, I wouldn't have any problem with coupling the SQL generator to the XML DOM. Save tomorrow for tomorrow.