2

I have a TableSqlBuilder interface which has a method getCreateTableSql. This interface is used in some other class as follows:

function createTable( $tableDefinition ) {
    $sql = $this->tableSqlBuilder->getCreateTableSql( $tableDefinition );
    $this->db->execute( $sql );
}

The TableSqlBuilder interface has multiple implementations, such as SQLiteTableSqlBuilder and MySQLTableSqlBuilder.

SQLiteTableSqlBuilder originally created all SQL itself, and had methods to create SQL for an index or for a field. It turned own that the functionality of these methods was needed on its own at other places, so it has been split off into a SQLiteFieldSqlBuilder and a SQLiteIndexSqlBuilder. These classes are instantiated by SQLiteTableSqlBuilder - they are thus not injected, and SQLiteTableSqlBuilder depends on these concrete classes.

I'm unsure if this should be changed to have these field and index sql builders injected. On the one hand this gives more flexibility. It also allows mocking out this behaviour in tests. Then again, the binding and flexibility is currently the same as before the code was split of into new classes.

When going with the injection approach, the accepted types could be FieldSqlBuilder and IndexSqlBuilder, both interfaces implemented by the current concrete classes. This however means one can happily end up with a MySqlIndexBuilder in a SQLiteTableBuilder, which is clearly to be avoided. Could also just type hint against the concrete class, pretending it is an interface, which gets rid of the dependency as long as the construction happens outside of the table sql builder.

Do you think these collaborators should be injected, or is it a valid exception where it is better to not go all the way?

2

It's a good idea to inject SQLiteFieldSqlBuilder and SQLiteIndexSqlBuilder specifically into SQLiteTableBuilder. If you decide you want to allow alternate implementations (e.g. for mocking), you can more easily refactor SQLiteIndexSqlBuilder and SQLiteFieldSqlBuilder into interfaces when necessary.

Injecting general FieldSqlBuilder and IndexSqlBuilder implementations into a SQLiteTableBuilder is not a good idea, since it doesn't properly represent its dependencies. The situation in which you would inject implementations of general interfaces like FieldSqlBuilder and IndexSqlBuilder would be for a general TableSqlBuilder class that works for both SQLite and MySQL field/index builders. This is a perfectly valid and recommendable approach, provided that the logic inside of TableSqlBuilder can be generalized for both cases.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.