Convert lots of method overloads into a generic method, design question

I'm trying to refactor some code here that was done previously by other guys, since i find it quite unpractical Here's an example

protected void SetParameterValue(SqlParameter parameter, string parameterValue, bool isNullable)
{
     if ((null == parameterValue || parameterValue == NULL_STRING) && isNullable)
            parameter.Value = DBNull.Value;
     else   parameter.Value = parameterValue;
}

protected void SetParameterValue(SqlParameter parameter, int parameterValue, bool isNullable)
{
     if (parameterValue == NULL_INT && isNullable)
            parameter.Value = DBNull.Value;
     else   parameter.Value = parameterValue;
}

protected void SetParameterValue( SqlParameter parameter, Int64 parameterValue, bool isNullable)
{
     if (parameterValue == NULL_LONG && isNullable)
            parameter.Value = DBNull.Value;
     else   parameter.Value = parameterValue;
}

Like those, there are a lot more. Now i needed to create one that accepts a new type (which doesn't have a method for it yet) and decided that maybe i could clean up a bit, make this better. my idea is to create something like

protected void SetParameterValue<T>(SqlParameter parameter, T parameterValue, bool isNullable)

ho开发者_Python百科wever, i don't know what's the best approach, what can i encapsulate inside this generic method and what will i need to do in separate methods as well. Is it worth it? or the "lots of methods" approach is fine? what would i gain from the generic one? thanks!

One way of removing the need for switches would be to use some kind of dictionary to hold delegates which determine what constitutes null for each possible type. Although I think you'd have to stick with object for this. So you'd have a dictionary and set it up like:

private Dictionary<Type, Func<object, bool, bool>> _nullChecks = new Dictionary<Type, Func<object, bool, bool>>();

private void SetupNullChecks(){
    _nullChecks.Add(typeof(string), (object parameterValue, bool isNullable) => { return (null == parameterValue || parameterValue.ToString() == NULL_STRING) && isNullable; });
    _nullChecks.Add(typeof(int), (object parameterValue, bool isNullable) => { return (int)parameterValue == NULL_INT && isNullable; });
    _nullChecks.Add(typeof(long), (object parameterValue, bool isNullable) => { return (long)parameterValue == NULL_LONG && isNullable; });
}

And your check would be like:

public void SetParameterValue(SqlParameter parameter, object parameterValue, bool isNullable)
{
    if (_nullChecks[parameterValue.GetType()].Invoke(parameterValue, isNullable))
        parameter.Value = DBNull.Value;
    else parameter.Value = parameterValue;
}

Although, as suggested by others, changing the code to use the Nullable type would be better.

You could always do

Protected void SetParameterValue(SqlParameter parameter, 
 object parameterValue, bool isNullable)....

parameter.Value takes an object so minus the validation for each type, you don't really need to seperate them out.

You could create a validate parameter method which reflects and pulls type type for the parameter and checks if the null value is set against that type. something like

bool IsNull (object value){
   if (value is int){
     check int..
   }
}
//this is a quick and dirty example, there are more elegant ways to handle it.

This condenses your type validation and all your overloads, and it removes the need for a generic method too.

The 'multiple signatures' approach to method definition is perfectly fine - it's what gives us polymorphism. Personally, I'd prefer to keep that technique rather than refactor as you suggest.

However, what I would do is replace the repetition of the method body in all but one with calls to the 'master' method, casting the parameter thus:

protected void SetParameterValue(SqlParameter parameter, int parameterValue, bool isNullable)
{
  SetParameterValue(parameter, (Int64)parameterValue, isNullable);
}
protected void SetParameterValue(SqlParameter parameter, Int64 parameterValue, bool isNullable)
{
  if (parameterValue == NULL_INT && isNullable)
    parameter.Value = DBNull.Value;
  else
    parameter.Value = parameterValue;
}

This presupposes that parameterValue can be re-cast without too much hassle, of course.

I think you can use nullable types instead of bool isNullable.

protected void SetParameterValue<T>(SqlParameter parameter, T parameterValue)
{
if (parameterValue == null)
  parameter.Value = DBNull.Value;
else
  parameter.Value = parameterValue;
}

And possibly call it accordingly with this signature. Like SetParameter(param,null) SetParameter(param,5)

It's difficult to "remove" the question with switch/if statments in the end, someone have to do it. You have the option of override/encapsulate what is null for an object or a class, but you'll still have to check what is null in each concept.

I don't know if this make things better, but you can first isolate the repetition by creating a method as said before or isolate only the nulls checking. The last one is what I did bellow:

protected void SetParameterValue(SqlParameter parameter,object parameterValue){
    if(IsParameterNull(parameterValue) && parameter.IsNullable){
        parameter.Value = DBNull.Value;
    }
    else{
        parameter.Value = parameterValue;
    }
}

List<NULLTYPE> nulls = new List<NULLTYPE>(){new NULLTYPE(NULL_INT), new NULLTYPE(NULL_LONG), new NULLTYPE(null)}
protected bool IsParameterNull(object parameterValue){
    if(nulls.Contains(parameterValue)) return true;
    else return false;
}

The job here is to create a NULLTYPE class that encapsulates your concepts of null and them the nulls.Contains(parameterValue) checks if the value exists in the list. You can go further and override Contains to check on your own way, but you have to think how much work do you want to spend on this.