I had a discussion with a collegue today, about how its best to return two values. We had three differing opinions. Important to the problem is, that we can't use an extra class or struct(therefore no Tuple) which was the prefered answer in this question: Clean Code - Are output parameters bad? .
public enum Offset
{
None = 0,
Training = 10,
Cancel = 20
}
static void Main(string[] args)
{
//Option 1
int outValue = 0;
Offset outOffset;
HandleOffset(35, out outValue, out outOffset);
//Option 2
int refValue = 0;
outOffset = SubtractOffset(ref refValue);
//Option 3
outOffset = GetOffsetById(35);
int id = GetIdWithoutOffset(35);
}
//Option 1
public static void HandleOffset(int inValue, out int outValue, out Offset outOffset)
{
outValue = inValue;
outOffset = Offset.None;
if ((inValue-(int)Offset.Cancel)>15)
{
outValue -= (int)Offset.Cancel;
outOffset |= Offset.Cancel;
}
if ((inValue - (int)Offset.Training) > 15)
{
outValue -= (int)Offset.Training;
outOffset |= Offset.Training;
}
}
//Option 2
public static Offset SubtractOffset(ref int id)
{
Offset offset = Offset.None;
if ((id - (int)Offset.Cancel) > 15)
{
id -= (int)Offset.Cancel;
offset |= Offset.Cancel;
}
if ((id - (int)Offset.Training) > 15)
{
id -= (int)Offset.Training;
offset |= Offset.Training;
}
return offset;
}
//Option 3
public static Offset GetOffsetById(int id)
{
Offset offset = Offset.None;
if ((id - (int)Offset.Cancel) > 15)
{
offset |= Offset.Cancel;
}
if ((id - (int)Offset.Training) > 15)
{
offset |= Offset.Training;
}
return offset;
}
//Option 3
public static int GetIdWithoutOffset(int id)
{
if ((id - (int)Offset.Cancel) > 15)
{
id -= (int)Offset.Cancel;
}
if ((id - (int)Offset.Training) > 15)
{
id -= (int)Offset.Training;
}
return id;
}
Option 2 seems ugly because of the return value + ref value.
Option 1 seems to be also ugly because of the two output parameters but in fact it looks cleaner than the second option.
Option 3 looks the cleanest to me.
Which of those solutions is regarded the cleanest solution( clean as in clean code by bob martin ), or is there another option to solve the problem, which we might not have thought of ?
Generally speaking, the most clean solution is the one not requiring any out or ref-parameters (best is a struct/class, tuples also work). Since you are not allowed to use classes/structs or tuples, which would be the preferred way, I'd still use the first option for being the easiest.
The second option, in my opinion, is complete bullshit, because the out-keyword is supposed to do exactly what you want, so using ref is not at all necessary.
The third option is okay, although very cluttered and not that easy. Since you cannot use tuples and want to maintain a clear code, avoid it.
TL;DR: Use the first option.
I'm just going to throw this out there, though I would never do this in production (at least for a public API).
An alternative option is to use a dynamic return type. No out param. No ref param. Only an obfuscation of the values being returned:
public class Program
{
public static void Main()
{
dynamic offset = HandleOffset();
Console.WriteLine(offset);
}
public static dynamic HandleOffset()
{
return new
{
Value = 64,
Offset = Offset.Cancel
};
}
}
public enum Offset
{
None = 0,
Training = 10,
Cancel = 20
}
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With