patterncsharpModerate
One instance per method call or multiple method call per instance
Viewed 0 times
permethodinstanceonecallmultiple
Problem
I have the following class designs, and I'm looking for the best practices, aligned to the OOP design principles/patterns.
The
Any comments regarding the method body will also be appreciated, though the focus of the question is on the class design/usage.
-
Use the constructor to take the parameter and parse (one instance per parse)
-
Parameterless constructor, multiple parsing per instance
-
A helper class, static method
I think the first one is more likely to be the best, but I can't explain which exactly are the advantages over the others. The question is "Considering design principles, which one is the best and why? Is there any pattern which would fit the scenario?".
The
ParameterParser will be used inside a foreach block through 10 items approximately.Any comments regarding the method body will also be appreciated, though the focus of the question is on the class design/usage.
-
Use the constructor to take the parameter and parse (one instance per parse)
public class ParameterParser
{
private readonly Type _propertyType;
private readonly string _parameter;
public ParameterParser(Type propertyType, string parameter)
{
_propertyType = propertyType;
_parameter = parameter;
}
public object Parse()
{
if (typeof(IEnumerable).IsAssignableFrom(_propertyType)
|| typeof(IEnumerable<>).IsAssignableFrom(_propertyType))
return _parameter.Split(new[] { ',', ';', '|' }, StringSplitOptions.RemoveEmptyEntries);
if (_propertyType.IsEnum)
return Enum.Parse(_propertyType, _parameter);
return Convert.ChangeType(_parameter, _propertyType);
}
}-
Parameterless constructor, multiple parsing per instance
public class ParameterParser2
{
public object Parse(Type propertyType, string parameter)
{
//same code
}
}-
A helper class, static method
public class ParameterHelper
{
public static object Parse(Type propertyType, string parameter)
{
//same code
}
}I think the first one is more likely to be the best, but I can't explain which exactly are the advantages over the others. The question is "Considering design principles, which one is the best and why? Is there any pattern which would fit the scenario?".
Solution
Option 1 is surprising.
I find it funny that you have
Option 2 is [more] consistent with the framework.
I don't mean to repeat what I just said, but it feels natural for a
How about keeping it type-safe and taking a generic type argument instead of a
This way if
Option 3 feels wrong.
Whenever I feel the need for a class to be called
I find it funny that you have
return Enum.Parse(_propertyType, _parameter); in a parameterless Parse() method. Enum.Parse is just an example (int.Parse would be another); these methods are commonly used by C# programmers and your single-use parser seems to break POLS in my opinion.Option 2 is [more] consistent with the framework.
I don't mean to repeat what I just said, but it feels natural for a
Parse method to take in all the parameters it needs. Now there is another problem here: your API is exposing object, which means the parsed value will need to be cast from the calling code, into the correct type. This is bad. Very bad. Nothing should ever be returning an object. If you're parsing a value type, you're boxing it right there. Is that not likely?How about keeping it type-safe and taking a generic type argument instead of a
Type parameter?public T Parse(string value)
{
...
return Convert.ChangeType(value, typeof(T));
}This way if
T is a value type, you're not boxing it. And you're not returning an object, and it's still the calling code that decides what type to parse the string with.Option 3 feels wrong.
Whenever I feel the need for a class to be called
xxxxxHelper, something itches. The class should be static as well, if it's only going to expose static methods. That said, as @Simon has noted (oh, well, Simon says...) this brings DI to its knees. Best stay clear from that.Code Snippets
public T Parse<out T>(string value)
{
...
return Convert.ChangeType(value, typeof(T));
}Context
StackExchange Code Review Q#37597, answer score: 10
Revisions (0)
No revisions yet.