Separate method or modify existing for functionality

June 7, 2022

I’m building method Convert to turn string of zeroes and ones into int:

public int Convert(string binaryString)
{
    ValidateOrThrow(binaryString);

    return ConvertInternal(binaryString); // Peforms convertion.
}

This method runs validation for provided string:

private void ValidateOrThrow(string binaryString)
{
    if (binaryString is null || binaryString.Length == 0)
        throw new InvalidDataException("Can't convert null or empty string");

    if (binaryString.Any(value => (byte)value is not 48 or 49))
        throw new InvalidDataException("Can't convert non-binary string");
}

Now I want to add new method TryConvert which returns bool as the result of Valdate instead of throwing an exception.

public bool TryConvert(string binaryString, out int result)
{
    if (!Validate(binaryString))
        return false;

    result = ConvertInternal(binaryString);
    return true;
}

I can create one more method Validate with bool return type, but it breaks DRY.

Or I can create single parameterized method and use it in both cases with different parameter:

private bool Validate(string binaryString, bool throwOnInvalidData)
{
    if (binaryString is null || binaryString.Length == 0)
    {
        if (throwOnInvalidData)
            throw new InvalidDataException("Can't convert null or empty string");
        else
            return false;
    }

    if (binaryString.Any(value => (byte)value is not 48 or 49))
    {
        if (throwOnInvalidData)
            throw new InvalidDataException("Can't convert non-binary string");
        else
            return false;
    }

    return true;
}

Is this valid way or I’m missing something?

>Solution :

I don’t like the nested if statements in your validate. What if you gave it an out parameter too –

private bool Validate(string binaryString, out string error)
{
     error = null;
     if (binaryString is null || binaryString.Length == 0)
     {
         error = "Can't convert null or empty string";
         return false;
     }

     if (binaryString.Any(value => (byte)value is not 48 or 49))
     {
          error = "Can't convert non-binary string";
          return false;
     }

     return true;
}

Then you have two methods that use it –

public int Convert(string binaryString)
{
    if(!Validate(binaryString, out var error))
    {
        throw new InvalidDataException(error);
    }

    return ConvertInternal(binaryString);
}

and

public bool TryConvert(string binaryString, out int result)
{
    result = 0;

    if (!Validate(binaryString, out var error))
    {
        return false;
    }

    result = ConvertInternal(binaryString);
    return true;
}