HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Class with TryParse static method

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
methodwithtryparseclassstatic

Problem

I am receiving and Expression (String) as follows:

String expression = "+name;-age;-city";


I parse this expression by splitting it using ";" and interpreting the +/- signs and create a List. To wrap this I created the following:

public class OrderExpression {

  public HashSet Rules { get; set; }

  public static Boolean TryParse(String expression, out OrderExpression orderExpression) {
    // Parse expression into a List
    if (_parser.ExpressionIsValid(expression)) {
       orderExpression = new OrderExpression { Rules = _parser.Parse(expression) }
       return true;
    else
       return false; 
  }
}


So I would use it as follows:

String expression = "+name;-age;-city";
OrderExpression orderExpression;
OrderExpression.TryParse(expression, out orderExpression);


Does this make sense? I am not sure if this architecture and naming is the way to go.

I am being picky about this because I will use it as a standard for an API to convert an order expression into a List.

Solution

The TryParse or TryGetValue pattern is often used in the .NET Class Library and is a good approach. In order to fully comply with it, you should always return the type's default value default(T) for the output parameter when the parsing fails. For reference types this is always null:

public static bool TryParse(string expression, out OrderExpression orderExpression) {
    if (_parser.ExpressionIsValid(expression)) {
        orderExpression = new OrderExpression { Rules = _parser.Parse(expression) };
        return true;
    } else {
        orderExpression = null;
        return false;
    }
}


An out parameter must be assigned before leaving the method anyway. You'll get a compiler error otherwise.

Also, in C#, the C# type aliases are usually preferred over the .NET type names. I.e.: use string instead of System.String, bool instead of System.Boolean, int instead of System.Int32 and so on. (Personally, I prefer to use the .NET type name when accessing static members: Int32.TryPare(...).)

When calling a method, the return value can be ignored in C#, but here, it seems logical to react differently, depending on the outcome of the parsing:

if(OrderExpression.TryParse(expression, out orderExpression)) {
    // Use orderExpression.
} else {
    // Handle the error case.
}


A comment regarding the logic: Order expressions usually have to be executed in a specific order, however, a HashSet is unordered. Return a List instead. Also, the setter can be private.

public List Rules { get; private set; }


UPDATE

Since C# 6.0 you can declare getter-only auto-properties. You can set them in the constructor or in a property initializer.

To set the Rules property I declare a constructor. It can be private to force a client to use the static TryParse factory method.

public class OrderExpression
{
    private OrderExpression(List rules)
    {
        Rules = rules;
    }

    public List Rules { get; }

    public static bool TryParse(string expression, out OrderExpression orderExpression)
    {
        if (_parser.ExpressionIsValid(expression)) {
            orderExpression = new OrderExpression(_parser.Parse(expression));
            return true;
        } else {
            orderExpression = null;
            return false;
        }
    }
}


Since C# 7.0 you can declare the out-parameter in-line

string expression = "+name;-age;-city";
if (OrderExpression.TryParse(expression, out var orderExpression)) {
    //TODO: use orderExpression.
} else {
    //TODO: handle pare error.
}

Code Snippets

public static bool TryParse(string expression, out OrderExpression orderExpression) {
    if (_parser.ExpressionIsValid(expression)) {
        orderExpression = new OrderExpression { Rules = _parser.Parse(expression) };
        return true;
    } else {
        orderExpression = null;
        return false;
    }
}
if(OrderExpression.TryParse(expression, out orderExpression)) {
    // Use orderExpression.
} else {
    // Handle the error case.
}
public List<OrderRule> Rules { get; private set; }
public class OrderExpression
{
    private OrderExpression(List<OrderRule> rules)
    {
        Rules = rules;
    }

    public List<OrderRule> Rules { get; }

    public static bool TryParse(string expression, out OrderExpression orderExpression)
    {
        if (_parser.ExpressionIsValid(expression)) {
            orderExpression = new OrderExpression(_parser.Parse(expression));
            return true;
        } else {
            orderExpression = null;
            return false;
        }
    }
}
string expression = "+name;-age;-city";
if (OrderExpression.TryParse(expression, out var orderExpression)) {
    //TODO: use orderExpression.
} else {
    //TODO: handle pare error.
}

Context

StackExchange Code Review Q#133188, answer score: 13

Revisions (0)

No revisions yet.