4
\$\begingroup\$

This is a fairly trivial example of a question I come up to often. In the example below I tend to think that allowing duplication sometimes results in clearer code.

Does anyone agree/disagree on this point?

First way: an additional local variable is used so method.Invoke is called just once on the last line. I think this has slightly more logic to decode than the second way.

private static void Invoke<T>(string[] args, MethodInfo method) where T : new()
{
    T target = new T();
    target.InvokeMethod("Initialize", errorIfMethodDoesNotExist: false, accessNonPublic: true);
    string[] methodArgs = null;
    if (args.Length > 1)
    {
        methodArgs = args.Skip(1).ToArray();
    }
    method.Invoke(target, methodArgs);
}

Second way: No additional local variable but the call to method.Invoke is duplcated in both branches of the if statement. I think this is clearer even though logic was duplicated.

private static void Invoke<T>(string[] args, MethodInfo method) where T : new()
    {
        T target = new T();
        target.InvokeMethod("Initialize", errorIfMethodDoesNotExist: false, accessNonPublic: true);
        if (args.Length > 1)
        {
            method.Invoke(target, args.Skip(1).ToArray());
        }
        else
        {
            method.Invoke(target, null);
        }
    }
\$\endgroup\$
  • \$\begingroup\$ I personally like your first option in this case as for me it reads easier and is actually less logic in my view to decode as such \$\endgroup\$ – dreza May 14 '12 at 23:04
8
\$\begingroup\$

I think this would be even better, in this case.

private static void Invoke<T>(string[] args, MethodInfo method) where T : new()
{
    T target = new T();
    target.InvokeMethod(
        "Initialize", errorIfMethodDoesNotExist: false, accessNonPublic: true);
    string[] methodArgs = args.Skip(1).ToArray();
    method.Invoke(target, methodArgs);
}

Skip() on an empty collection returns an empty collection.

| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

or if you want to keep the null value, you can inline the things in the method call with a ternary if like this :

method.Invoke(target, args.Length > 1 ? args.Skip(1).ToArray() : null);

Not super effective here considering svick answer, but in a more general case I think it is preferable to your two options (as long as it doesn't makes you write a 200 characters line).

| improve this answer | |
\$\endgroup\$

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.