Your second example has the benefit of no case statement, and no CommandType. This means you can a new statement type without having to modify a case statement. A case statement ought to cause an OOP programmer to pause a moment and consider whether the design should be changed to get rid of the case statement. A variable denoting a thing's type ought to cause the programmer to pause a long while.

Simpler than either of those approaches would be to use a class hierarchy, as you did, but then put the SQL generation inside that hierarchy rather than having it be a separate class:

class SqlCommand { abstract String sql; } class InsertCommand : SqlCommand { overrides String sql {...}; } class DeleteCommand : SqlCommand { overrides String sql {...}; } 

The code inside the sql functions will probably turn out to have shared behavior, or enough complexity, that moving the shared behavior and complexity into a separate SqlBuilder class would be good:

class InsertCommand : SqlCommand { overrides String sql { builder = SqlBuilder.new builder.add("insert into") builder.add_identifier(table_name) buidler.add("values") builder.add_value_list(column_names) ... return builder.sql }; } 

The SqlCommand drives the process of creating the sql, pushing its details into the SqlBuilder. This keeps the SqlCommand and SqlBuilder decoupled, since the SqlCommands do not have to expose their entire state for the builder to use. This has the benefits of your second example, but without the complexity of the visitor pattern.

Your second example has the benefit of no case statement, and no CommandType. This means you can a new statement type without having to modify a case statement. A case statement ought to cause an OOP programmer to pause a moment and consider whether the design should be changed to get rid of the case statement. A variable denoting a thing's type ought to cause the programmer to pause a long while.

However, the visitor pattern doesn't seem to be needed here. Simpler would be to use a class hierarchy, as you did, but then put the SQL generation inside that hierarchy rather than having it be a separate class:

class SqlCommand { abstract String sql; } class InsertCommand : SqlCommand { overrides String sql {...}; } class DeleteCommand : SqlCommand { overrides String sql {...}; } 

The code inside the sql functions will probably turn out to have shared behavior, or enough complexity, that moving the shared behavior and complexity into a separate SqlBuilder class would be good:

class InsertCommand : SqlCommand { overrides String sql { builder = SqlBuilder.new builder.add("insert into") builder.add_identifier(table_name) buidler.add("values") builder.add_value_list(column_names) ... return builder.sql }; } 

The SqlCommand drives the process of creating the sql, pushing its details into the SqlBuilder. This keeps the SqlCommand and SqlBuilder decoupled, since the SqlCommands do not have to expose their entire state for the builder to use. This has the benefits of your second example, but without the complexity of the visitor pattern.

If it turns out that sometimes you need the sql built one way and sometimes another (perhaps you want to support more than one SQL back end), then you abstract the SqlBuilder class, like so:

class SqlBuilder { abstract void add(String s); abstract void add_identifier(String identifier); abstract void add_value_list(String[] value_list); ... } class PostgresSqlBuilder : SqlBuilder { void add(String s) {...} void add_identifier(String identifier) {...} void add_value_list(String[] value_list) {...} ... } class MysqlSqlBuilder : SqlBuilder { void add(String s) {...} void add_identifier(String identifier) {...} void add_value_list(String[] value_list) {...} ... } 

Then the sql functions use the builder passed into them, rather than creating one:

class InsertCommand : SqlCommand { overrides String sql (SqlBuilder builder) { builder.add("insert into") builder.add_identifier(table_name) buidler.add("values") builder.add_value_list(column_names) ... return builder.sql }; } 

Simpler than either of those approaches would be to use a class hierarchy, as you did, but then put the SQL generation inside that hierarchy rather than having it be a separate class:

class SqlCommand { abstract String sql; } class InsertCommand : SqlCommand { overrides String sql {...}; } class DeleteCommand : SqlCommand { overrides String sql {...}; } 

If the code inside the sql functions turns out to have shared behavior, then you can put the shared behavior into an SqlBuilder class, which each sql function then uses, e.g.:

class InsertCommand : SqlCommand { overrides String sql { builder = SqlBuilder.new builder.add("insert into") builder.add_identifier(table_name) buidler.add("values") builder.add_value_list(column_names) ... return builder.sql }; } 

This differs from your second example in that the SqlCommand is driving the process of creating the sql, pushing its details into the SqlBuilder, rather than having the generator pull details from the SqlCommand. This keeps the SqlCommand and SqlBuilder decoupled, since the SqlCommands do not have to expose their entire state for the builder to use.

This structure also makes it easy to add new sql commands without having to modify any "which type of SQL statement is it?" case statements, helping to satisfy the single responsibility principle. Every case statement in an OOP program should cause the programmer to pause a moment and wonder if the design can be improved.

Your second example has the benefit of no case statement, and no CommandType. This means you can a new statement type without having to modify a case statement. A case statement ought to cause an OOP programmer to pause a moment and consider whether the design should be changed to get rid of the case statement. A variable denoting a thing's type ought to cause the programmer to pause a long while.

Simpler than either of those approaches would be to use a class hierarchy, as you did, but then put the SQL generation inside that hierarchy rather than having it be a separate class:

class SqlCommand { abstract String sql; } class InsertCommand : SqlCommand { overrides String sql {...}; } class DeleteCommand : SqlCommand { overrides String sql {...}; } 

The code inside the sql functions will probably turn out to have shared behavior, or enough complexity, that moving the shared behavior and complexity into a separate SqlBuilder class would be good:

class InsertCommand : SqlCommand { overrides String sql { builder = SqlBuilder.new builder.add("insert into") builder.add_identifier(table_name) buidler.add("values") builder.add_value_list(column_names) ... return builder.sql }; } 

The SqlCommand drives the process of creating the sql, pushing its details into the SqlBuilder. This keeps the SqlCommand and SqlBuilder decoupled, since the SqlCommands do not have to expose their entire state for the builder to use. This has the benefits of your second example, but without the complexity of the visitor pattern.