2
\$\begingroup\$

For our (internal) automated test project, I am writing a part where parameters can be passed to an SSIS Package before it gets executed. The tester delivers the parameter name, value and desired datatype in a table format. I get the datatype and parses it to the correct type. We assume a happy flow (when testers do something wrong, no pretty feedback is needed, they can read the exceptions).

The following code is the snippet I am not happy with:

foreach (var row in parametersTable.Rows)
{
    object value;
    if (parametersTable.ContainsColumn("DataType") && !String.IsNullOrEmpty(row["DataType"]))
    {
        string dataType = row["DataType"].ToLower();
        if ("int64".Equals(dataType))
        {
            value = Int64.Parse(row["Value"]);
        }
        else if ("datetime".Equals(dataType))
        {
            value = DateTime.Parse(row["Value"]);
        }
        else
        {
            throw new ArgumentException("DataType " + row["DataType"] + " is not supported");
        }
    }
    else
    {
        value = row["Value"];
    }

    pfe.AddParameter(row["Name"], value);
}

I think, my string manipulation and testing can be improved and I'd like to make the "test and parse" section more expandable. Maybe an Enum or an helper object where I just have to add a single value to make it work for additional datatypes.

\$\endgroup\$
  • \$\begingroup\$ What type of object is pfe ? \$\endgroup\$ – Heslacher Oct 31 '14 at 10:35
  • \$\begingroup\$ Own created object: "PackageFileExecuter". The AddParameter creates a parameter of type PackageInfo.ExecutionValueParameterSet and adds it to a collection of parameters that is passed to the PackageInfo instance before it gets Executed. The ParameterValue property is of type Object, so I can (and have to) pass it different datatypes. \$\endgroup\$ – AutomatedChaos Oct 31 '14 at 10:45
  • \$\begingroup\$ Is the values stored in the DataType column just some own created string or is it a SqlDbType like d347hm4n assumes? \$\endgroup\$ – Heslacher Oct 31 '14 at 10:57
  • \$\begingroup\$ The .NET framework has a Convert.ChangeType method which may be more suitable to use instead of the type explicit method like Int64.Parse. msdn.microsoft.com/en-us/library/dtb69x08(v=vs.110).aspx \$\endgroup\$ – hocho Oct 31 '14 at 19:22
2
\$\begingroup\$

There is no magic method in .NET that will know the datatype and magically returns the right parsed value for you. So, creating your own method of parsing and doing manipulation is not a bad thing. This can be improved though.

Use a method:

I prefere to place code that does a certain manipulation or calculation on variables in a separate method/function. This gives me the ability to reuse the code if necessary and maintaining the code is also easier then looking in chunks of code to change just a small bit (like adding a datatype in your case).

switch vs. if/else:

When facing a simple check for a value or condition of a variable, an if/else-statement will certainly do. But when you have a list of options for a variable, it's better to use a swicth-statement. Your code not only looks cleaner, it is cleaner and again, easier to maintain.

This results in following code:

public object ParseValue(string type, string value)
{
    switch(type)
    {
        case "int64" : return Int64.Parse(value);
        case "datetime" : return DateTime.Parse(value);
        case "double" : return Double.Parse(value);
        default: throw new ArgumentException("DataType is not supported");
    }
}

And the usage:

foreach (var row in parametersTable.Rows)
{
    object value;

    if (parametersTable.ContainsColumn("DataType") && !String.IsNullOrEmpty(row["DataType"]))
    {
        var dataType = row["DataType"].ToLower();
        value = ParseValue(dataType, row["Value"]);
    }
    else
    {
        value = row["Value"];
    }

    pfe.AddParameter(row["Name"], value);
}

Another way of using a method is making it return a bool to see if the parsing succeeded and using the out parameter. This results in following method and usage:

public bool TryParseValue(string type, string value, out object parsedValue)
{
    switch(type)
    {
        case "int64" : parsedValue = Int64.Parse(value); return true;
        case "datetime" : parsedValue = DateTime.Parse(value); return true;
        case "double" : parsedValue = Double.Parse(value); return true;
        default: parsedValue = null; return false;
    }
}

foreach (var row in parametersTable.Rows)
{
    object value;

    if (parametersTable.ContainsColumn("DataType") && !String.IsNullOrEmpty(row["DataType"]))
    {
        var dataType = row["DataType"].ToLower();
        if(!TryParseValue(dataType, row["Value"], out value))
        {
            throw new ArgumentException("DataType is not supported");
        }
        //'value' will be correct if the parsing succeeeded
    }
    else
    {
        value = row["Value"];
    }

    pfe.AddParameter(row["Name"], value);
}
\$\endgroup\$
  • \$\begingroup\$ Thank you for the comprehensive answer. I made it myself difficult by thinking of Java (pre version 7) switches that could not take strings. This works nicely in my solution and having a single point to add another datatype is a great feature. \$\endgroup\$ – AutomatedChaos Oct 31 '14 at 12:02
-1
\$\begingroup\$

You are going to want to use SqlDbType in System.Data.

Something like this:

SqlDbType dataType = row["DataType"] as SqlDbType;
switch(dataType)
{
   case SqlDbType.Int:
      value = Int64.Parse(row["Value"]);
      break;
   case SqlDbType.Int:
      DateTime.Parse(row["Value"]);
      break;
}

Hope that helps.

\$\endgroup\$
  • \$\begingroup\$ -1 If you suggest something, please do it in a clean bug free way. \$\endgroup\$ – Heslacher Oct 31 '14 at 10:41
  • \$\begingroup\$ In fairness, the supplied sample isn't "Working" code either. \$\endgroup\$ – d347hm4n Oct 31 '14 at 11:27
  • \$\begingroup\$ Why do you think so? \$\endgroup\$ – Heslacher Oct 31 '14 at 11:27

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.