Any performance issues with this code

this is probably going over the top, but I was after a function that assigns a value to my class property if the value from the database is not null and do nothing if it is. I could do this with if statements but I was after a one liner. I want this to go everywhere in my code but before doing so was just wondering if this is the right way to go about it with performance in mind. Thanks.

Calling code: assign(variable, value)

Utils.Assign(dataSet.Tables[0].Rows[0]["Assigned"], ref this.Assigned);

Utils Functions:

public static void Assign(object value, ref Boolean variable)
{
    if (IsRealString(value)) // Check 
    {
        switch (IsBoolean(value.ToString())) { case t开发者_运维问答rue: variable = Convert.ToBoolean(value); break; }
    }
}

public static Boolean IsBoolean(String s)
{
    Boolean bit;
    if (Boolean.TryParse(s, out bit))
        return true;
    else
        return false;
}

public static bool IsRealString(object s)
{
    return s != null && !String.IsNullOrEmpty(s.ToString());
}

Nothing major, but you convert value to a boolean twice and use a switch statement as an if statement. The real problem here is readability, not performance. I recommend simplifying things:

public static void Assign(object value, ref bool variable)
{
    if (value == null) return;

    bool result;
    if (Boolean.TryParse(value.ToString(), out result))
    {
        variable = result;
    }
}

There are a few issues with the code that go well beyond performance. First, worry about correctness and readability, and then you can worry about performance -- and that should be done by measuring / profiling. It is highly unlikely this kind of code will be the bottleneck, so to start by "optimizing" here is unlikely to be worth your time.

Note that IsRealString will not return a reliable answer. The method takes an object, so the default ToString for any object will give some non empty string, which will return true.

You stated you want:

a function that assigns a value to my class property if the value from the database is not null and do nothing if it is

You can clarify this a little:

1. If the value from the database is null, do nothing
2. Otherwise, assign the value to my class property

So now you've got a concise and clear definition for what you want your function to do. If your implementation follows that description, it is likely to be simple and correct, which is a great starting point.

So lets try it:

public static void Assign(object value, ref bool variable)
{
    if (value == null) return;

    // We're assuming that a valid value will always be a string.  If this is not the
    // case, then we need to handle other types

    string valueString = value as string;

    bool result;
    if (bool.TryParse(valueString, out result))
    {
        variable = result;
    }
}

Note: this is the same as Greg's sample, except I'd avoid calling ToString(), since we don't know what the object's implementation does.

We also discover one problem with the definition of our "spec". The assumption in your (and my) code, is that if a value is "set", it will always be set to a string. You could have a dataset where the values are not strings. If, for example, the value was a boolean, parsing valueString will not give a valid result. This really depends on what your dataset contains.

Is there some reason you're doing a switch statement over a boolean type? That's what if statements are for. I see no performance-sensitive code in here to worry about at all. Go solve bigger problems that are real problems and stop chasing ghosts.

If there shoud be any performance issues, you should be able to measure them with a profiler. Before you don't measure them, don't try to improve performance. As you are encapsulating this code, you would still be able to improve it easily.

But note that if you have a database, the database will be your bottleneck and not this part of code. Definitly!

The reason why you should NOT try to speed up the code is because it usually makes code less readable. However, you can follow the suggestions others made (if instead of switch, simplify IsRealString()) and also think about if you really need the IsBoolean() or maybe just call Boolean.TryParse from Assign() because this would infact improve readability!

i would write the IsRealString method as follow :

public static bool IsRealString(object s)
{
    return String.IsNullOrEmpty(s);
}

but of course this brings no real improvement in terms of speed :)

and the second function :

public static Boolean IsBoolean(String s)
{
    Boolean bit = false;
    return Boolean.TryParse(s, out bit);
}