Method design - clarity or multifunction

Question

I've made a class in both Java and C# that allows me to perform SQL queries, as an example I have a method called Delete which accepts several parameters;

public static int Delete(String table, String column, String operand, Object value)

I have the value as an Object type as I may want to delete rows based on String, Integers or booleans, this means the method is flexible enough to support the different types. I then append additional " ' " characters to the query depending on whether it is a string or not by using the instanceof test in Java (or .GetType in C#)

Crude example:

if (value instanceof String) 
{   
    System.out.println("It's a String");   
}   
else 
{   
    System.out.println("It's not a String");   
}

When implementing the rest of the class I began to think to myself whether the previously mentioned method is an ideal way or whether I should implement additional methods for specific data types, one to accomodate String, another for Integer and so on.

If I went about implementing this then it would mean that there would be additional methods with mimnimal difference in logic, however each method only has one purpose making them simple to follow and document. On the other hand if I keep it the way it is then there is a lot less code to be produced and maintained, it can manage any type of Delete statement (in terms of data types) and there only needs to be a few if statements to determine the type of object that was passed in via the parameter.

Which is the best approach to follow from a object-orientated / best code practices point of view?

Thanks for the information!

Don't use a comparision on `.GetType`, use the `is` operator. `is` compiles to a IL instruction, while `GetType` is a very expensive call. — Femaref, Mar 30 '11 at 18:02

score 5 · Accepted Answer · edited Jun 20 '20 at 09:12

5

Neither.

You must use parameterized queries.

You're right, though; you should keep it in a single method.
In C#, it is sometimes useful to make such methods generic.

edited Jun 20 '20 at 09:12

Community

1
1

answered Mar 30 '11 at 18:01

SLaks

868,454
176
1,908
1,964

1

Yes yes a thousand times yes - avoid SQL injection with parameterized queries! – n8wrl Mar 30 '11 at 18:09
Thanks for the advice, looking at some documentation seems to mention that it also gives better performance as well. – Jamie Keeling Mar 30 '11 at 18:12
Are there any good resources on using parameterized queries? My initial searches for parameterized queries with Java and C# (System.Data.SQLite) returns barely anything useful for a novice to use. – Jamie Keeling Mar 30 '11 at 18:32
I was a bit hesitant to upvote: I think you didn't make your text big and bold enough :P – Edurne Pascual Mar 30 '11 at 18:48
SQL injection must be avoided. – Michael Riley - AKA Gunny Mar 30 '11 at 19:03
I've managed to get around having to check data types by simply using the .setObject(); method for the prepared statement. Is this suitable or should I specifically select the methods for the correct type e.g. .setString() for a string. – Jamie Keeling Mar 30 '11 at 20:47

score 2 · Answer 2 · answered Mar 30 '11 at 18:14

Setting aside the not-asked question of whether this architecture is a good idea...

Generally, object oriented code is made better by removing, as much as possible, explicit type checking. This would imply that—to the extent that it matters which type is supplied as a parameter—the version of the code with type-specific overloads may be better. This is only an improvement, however, if the type of the parameter is known at compile-time, of course, since that is when overload resolution is done!

Also, in the C# version, method overloads will avoid boxing the value types.

Another, (in this case) possibly contradictory, rule is that duplicate code should be removed as much as possible. This means that the best way to do this may be to have a single method. In that case, I would recommend farming out the type-specific code to other methods (string DelimitValueIfNecessary(object)) since such things are not the core-competency of a method that constructs delete statements.

I think this second rule is more important than the first, so I would opt for a single method.

Now to go with the unasked question about this architecuture: This is a terrible idea for a number of reasons, not limited to: SQL injection attacks, tight coupling of object-model and data-model datatypes, innefficiency, leaky abstractions, etc.

Method design - clarity or multifunction

2 Answers2

You must use parameterized queries.

Linked