Reputation: 127
I need to change the "out" to "return" because it would be more comfortable to use, but I don't really know how to transform these functions to use "return" not "out". I have public class and in this class I have bool method
public static bool GetParameter(this Dictionary<string, string> context, string parameterName, out int parameter)
{
string stringParameter;
context.TryGetValue(parameterName, out stringParameter);
return int.TryParse(stringParameter, out parameter);
}
and I use this function like this:
private int _personID;
public void SomeFunction()
{
_classInstance.Context.GetParameter("PersonID", out _personID);
}
Thanks!
Upvotes: 1
Views: 1251
Reputation: 109567
Firstly, I want to say that the vast majority of the time, you should handle failure conditions by throwing exceptions.
However, very occasionally you want to be able to call a method to obtain a value where that operation may fail, but you don't want to throw an exception to express that failure.
This is obviously a general concept, and the standard .Net library addresses this using the TryXXX()
pattern.
I personally don't like this because it makes the code harder to read and it interferes with delegates, async and fluent interfaces.
Therefore we wrote our own wrapper class to handle these cases. Here's a cut-down example (the real code has a lot of code contracts, and also allows optional descriptive error strings and exceptions):
public class Result<T>
{
public static Result<T> Success(T value)
{
return new Result<T>(value, true);
}
public static Result<T> Failure()
{
return new Result<T>(default(T), false);
}
private Result(T value, bool succeeded)
{
_value = value;
_succeeded = succeeded;
}
public T Value
{
get
{
// Don't call this on failure!
Trace.Assert(_succeeded);
return _value;
}
}
public bool Succeeded
{
get
{
return _succeeded;
}
}
private readonly T _value;
private readonly bool _succeeded;
}
For your case you would use it something like this:
public static Result<int> GetParameter(this Dictionary<string, string> context, string parameterName)
{
string stringParameter;
context.TryGetValue(parameterName, out stringParameter);
int result;
if (int.TryParse(stringParameter, out result))
return Result<int>.Success(result);
else
return Result<int>.Failure();
}
And the calling code would look like:
var personIdRetrieval = _classInstance.Context.GetParameter("PersonID");
if (personIdRetrieval.Succeeded)
_personID = personIdRetrieval.Value;
else
// Handle failure.
Upvotes: 0
Reputation: 13286
I can think of two good options, if you aren't willing to use the out
parameter. I should say, though, that from your question, it looks like that's the best way to do it.
There are only really two reasons I generally see to avoiding the try-out model like this:
Aside from that, it's a lovely model to use that conveys a lot of information without sacrificing data integrity (or making any assumptions about what you might be expecting).
The issue here, and the reason you return a bool
as it currently is, is handling errors. So, you need to find an alternative way of handling them.
The choice here is really dependent on what kind of input you're expecting.
Use an exception:
The simplest, probably, is just to not handle them. Let them propagate up. If something is not found or not parsable, just throw an exception.
Using exceptions to guide regular application flow is generally accepted to be bad practice, but it can be up for interpretation what "regular application flow" is. So definitely do look at your data and circumstances.
public static int GetParameter(this Dictionary<string, string> context, string parameterName)
{
string stringParameter = context[parameterName];
return int.Parse(stringParameter);
}
Use null:
If you're expecting exceptions to be more or less common-place, you can return null, and just set your contract to use null
when something illegal happens. Be careful to handle that on the calling side, though!
An approach similar to this is used for many IndexOf
functions, like that on string
. They return -1 rather than null, but the principle is the same--have one value that you know will never occur in actual data, and set your contract up such that it means "this didn't work."
This is kind of what I was thinking of when I mentioned data integrity and assumptions before. What if you wanted to return that yes, the dictionary does include an empty string, and that should mean a null int. Suddenly, you're no longer able to convey that. So yes, it works, but that's a decision you have to remember. Make sure your "fail" case will never be the result of a successful pass.
public static int? GetParameter(this Dictionary<string, string> context, string parameterName)
{
string stringParameter;
if (!context.TryGetValue(parameterName, out stringParameter))
return null;
int ret;
if (!int.TryParse(stringParameter, out ret))
return null;
return ret;
}
Return a concrete type:
This one requires some overhead, but it has all the niceness of an out
parameter, without actually requiring it.
That said, I'm not sure whether I actually like this all that much or not. It's great in what it gives you, but it just feels very heavy to me for what you're using it for. But in any event, it is another option.
public class ParseResult
{
public ParseResult(bool IsSuccess, int Result)
{
this.IsSuccess = IsSuccess;
this.Result = Result;
}
public bool IsSuccess { get; set; }
public int Result { get; set; }
}
public static ParseResult GetParameter(this Dictionary<string, string> context, string parameterName)
{
int ret;
string stringParameter;
if (context.TryGetValue(parameterName, out stringParameter)
&& int.TryParse(stringParameter, out ret))
{
return new ParseResult(true, ret);
}
else
{
return new ParseResult(false, 0);
}
}
Upvotes: 3
Reputation: 121
Like this?
public static int GetParameter(this Dictionary<string, string> context, string parameterName)
{
string stringParameter;
context.TryGetValue(parameterName, out stringParameter);
int parameter;
int.TryParse(stringParameter, out parameter);
return parameter;
}
private int _personID;
public void SomeFunction()
{
_personID = _classInstance.Context.GetParameter("PersonID");
}
Upvotes: 0
Reputation: 56697
You could do the following:
public static int? GetParameter(this Dictionary<string, string> context, string parameterName)
{
string stringParameter;
if (!context.TryGetValue(parameterName, out stringParameter))
return null;
int value;
if (!Int32.TryParse(stringParameter, out value))
return null;
return value;
}
This makes the method return a nullable int
. null
would be returned in case of errors, otherwise the converted int
is returned.
Use it like:
int? value = GetParameter(...);
if (value == null)
// error
else
// use value.Value to access the real int.
Upvotes: 0
Reputation: 26268
You can transform it this way, if you know the parameter always exists:
public static string GetParameter(this Dictionary<string, string> context,
string parameterName)
{
string stringParameter = context[parameterName];
return int.Parse(stringParameter);
}
Upvotes: 0
Reputation: 2323
It seems that you don't use the bool result of the method, so then you can simply edit your method to this:
public static int GetParameter(this Dictionary<string, string> context, string parameterName)
{
int parameter;
string stringParameter;
context.TryGetValue(parameterName, out stringParameter);
int.TryParse(stringParameter, out parameter);
return parameter;
}
Upvotes: 0