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?