I have the following class, this class like many rely one parameters coming in as a pair.
Originally for convenience, I set them as params Object[] values and check if there is an even number of them if (values % 2 == 0).
Example Code:
using RVD = System.Web.Routing.RouteValueDictionary;
/// <summary>
/// Allows for quick building of similar RVDs.
/// </summary>
public class RVDBuilder
{
public RVD Template { get; protected set; }
public RVDBuilder(RVD template)
{
this.Template = template;
}
public RVDBuilder(params Object[] routeValues)
{
if (routeValues.Length % 2 != 0)
{
throw new Exception("parameters must come in pairs of two!");
}
}
}
I can see two point of views here:
- This adds great convience for the programmer: i.e.
new RVDBuilder("Key1", 1, "Key2", "2", ...); - This is not as safe as it could be: i.e.
new RVDBuilder(Tuple.Create("Key1", 1), ...);
I was hoping to get input on this subject/topic.
- Should I force the programmer to use a ‘pair’ class? (How would this affect coupling?)
- Should I make it safer even though It’ll be an easy fix/catch if an error is made?
- Is throwing an exception the best way to handle it or should I just truncate?
- Would adding an event so the programmer can choose how to handle the situation be good practice?
Overall though, how do I identify and decide on a method for handling this situation? I know overall it depends on the setting of the project, but there’s not really a mood/convention set for it on this subject. Are there any general rule-of-thumbs?
(Sorry if this isn’t quite the right StackExchange for this question. It involves code, but I figured since it’s purely for example to help better explain my design question it fit best here).
I had to tackle this recently in Java. My solution was to make the Pair class fairly painless to instantiate: Pair.of(..., ...). That’s only 7 extra characters before the parentheses, and naming the factory method of seems to be a convention that value-based classes follow in Java 8.
But in my use case there were usually no more than 3-4 pairs, so I also provided overloads for up to 5 pairs of arguments, so the tuples rarely need to be used. It’s mindless boilerplate to write, but it only needs to be done once and the callers benefit from it many times.
Should I force the programmer to use a ‘pair’ class? (How would this affect coupling?)
A “pair” class is a fairly harmless dependency. It has a small, well-defined interface and semantics, and a simple and straightforward implementation. It’s very unlikely that its interface will ever change, or that someone will break it in the future. It’s also very unlikely to depend on other classes.
Is throwing an exception the best way to handle it or should I just truncate?
Do not truncate. This is clearly an error on the programmer’s part, and you can’t even assume that the missing value is at the end: I once accidentally fused two strings around the middle of the argument list, e.g. "foo", "bar", "baz", "quux" became "foo", "bar baz", "quux". There’s nothing for you to do but abort with an exception.
Would adding an event so the programmer can choose how to handle the situation be good practice?
There’s no reason for the programmer to deliberately pass in a bad argument list, so if the error is handled at all, it’ll probably happen at a high level “catch all” error handler, not at the method call. The caller can already catch the exception if he so wishes, so the event won’t help.
0
I read in “Effective C++” that APIs should always be easy to use correctly and hard to use incorrectly. Since you’ve stated that it can be used incorrectly, I would ditch it for a solution that can be safer.
2
Your class is called RVDBuilder. The point of the builder pattern is to avoid ridiculously complex constructors and instead allow you to gradually collect all necessary information via ordinary method calls. Therefore, the best solution would be to add a fluent interface to add pairs of information:
class RVDBuilder {
public RVDBuilder Add(string key, Object value) { … }
public RVD Instantiate() { … }
}
Then: new RVDBuilder().Add("foo", 42).Add("bar", "baz").Instantiate().