Problem
Here is a method I have:
private void ProcessExpiryDate(out DateTime cashDate, out DateTime expiryDate, out DateTime expiryDateSecond)
{
// Lots of logic here, that's why I created this method
}
This private method is called several times in other public methods.
And every time a call is needed, I need to do the following codes:
DateTime cashDate;
DateTime expiryDate;
DateTime expiryDateSecond;
ProcessExpiryDate(out cashDate, out expiryDate, out expiryDateSecond);
existingObject.CashDate = cashDate;
existingObject.ExpiryDate = expiryDate;
existingObject.ExpiryDateSecond = expiryDateSecond;
Is there any better suggestion to make the lines shorter during the call ?
These 7 lines I need to write in every call looks ugly.
Solution
It’s preferable to not allow out parameters in your code.
Methods should be immutable as far as possible and not change the state of incoming data.
I would:
- Return an object that encapsulates
cashDate
,expiryDate
,expiryDateSecond
(ex:ProcessedExpiryDate
object) - Use a method such as SetDates as Maxim had suggested
This will allow you to turn your method call into:
SetDates(existingObject, ProcessExpiryDate())
You’re not the only one who noticed this problem. With C#7 they give us a better way to do it, with inline out parameters:
ProcessExpiryDate(out DateTime cashDate,
out DateTime expiryDate,
out DateTime expiryDateSecond);
But if you’re not ready to leap into version 7, you could just pass the object in and let the method set the dates. Or better yet, define an interface.
interface IExpirable
{
DateTime CashDate { set; }
DateTime ExpiryDate { set; }
DateTime ExpiryDateSecond { set; }
}
class ExistingObject : IExpirable
{
//Define as you did before
}
private void ApplyExpiryDate(IExpirable o)
{
o.CashDate = moe;
o.ExpiryDate = larry;
o.ExpiryDateSecond = curly;
//No need to return the modified object, because it was passed by reference
}
void MainCodeExample(ExistingObject existingObject)
{
ApplyExpiryDate(existingObject);
}
Notes:
-
In keeping with common standards I switched from camelCase to PascalCase for the property names.
-
I changed “Process” to “Apply” in the method name, which I think is more likely to be understand as a method which changes the object that is passed in.
Why not to turn your ProcessExpiryDate
method to this?
private void SetDates(SomeObject existingObject)
{
// calculate dates
existingObject.CashDate = cashDate;
existingObject.ExpiryDate = expiryDate;
existingObject.ExpiryDateSecond = expiryDateSecond;
}
And all your code will be turned into
SetDates(existingObject);