-1

I want to let a user specify a custom color for a class in the constructor via passing in RGBA bytes. If they do not specify a custom color, a default color from the application settings will be used. If the Alpha is left out, however, assume fully opaque.

What I would like:

public MyClass(byte r_col = -1, byte g_col = -1, byte b_col = -1, byte a_col = 255)
{
    if (r_col == -1 | g_col == -1 | b_col == -1)
    {
        // use default color
    }
    else
    {
        this.color = System.Windows.Media.Color.FromArgb(a_col, r_col, g_col, b_col); 
    }
}

However, there is no "wrong" value for a byte (-1 is invalid), so I am unable to detect if a byte was actually passed into the function. What options do I have? I'd like to avoid function overloading if possible.

Michael
  • 425
  • 2
  • 9
  • 2
    Actually this is bad API design, because you allow the caller to pass some values that will be ignored. E.g. what will happen if caller passes only R and G? They should only be allowed to pass nothing, RGB or RGBA, and for that you need overloads – P. Kouvarakis Apr 09 '17 at 22:34
  • Good point - for now it uses the default color (and does not give any error). I suppose overloading would be the most complete method here. – Michael Apr 09 '17 at 22:51

2 Answers2

1

Function overloading is much more beautiful in this case:

public MyClass()
{
    //Default color
}
public MyClass(byte r_col, byte g_col, byte b_col)
{
    color = Color.FromArgb(r_col, g_col, b_col);
}
public MyClass(byte a_col, byte r_col, byte g_col, byte b_col)
{
    color = Color.FromArgb(a_col, r_col, g_col, b_col);
}

Of course it is possible to do it without (as Micheal proofed), but it's (P.Kouverakis mentioned) not good API design. Because if you let the user type in parameters, which aren't allowed, this may result in difficult to trace bugs. Never fear more work for a greater result - so in this case, use function overloads.

MetaColon
  • 2,895
  • 3
  • 16
  • 38
0

I suppose this is one of the reasons why C# has nullable types. The following code worked well by using a nullable type to check if usable arguments were passed in.

public MyClass(byte? r_col = null, byte? g_col = null, byte? b_col = null, byte a_col = 255)
{
    if (r_col == null | g_col == null | b_col == null)
    {
        // use default color
    }
    else
    {
        System.Windows.Media.Color.FromArgb(a_col,
                                            r_col.GetValueOrDefault(),
                                            g_col.GetValueOrDefault(),
                                            b_col.GetValueOrDefault());
    }
}

This is an answer to my own question - other suggestions are also appreciated.

Michael
  • 425
  • 2
  • 9
  • 1
    Actually I don't think the `GetCalueOrDefault()` method is neccessary here - `.Value` should do it, as you check it before. But why exactly are you using only one vertical bar instead of two? I'm not sure wether this would work... – MetaColon Apr 09 '17 at 22:46
  • While it works either way, I think your suggestions would result in slightly faster execution. – Michael Apr 09 '17 at 23:04