Skip to content

Library design is fundamentally flawed

One of the goals of this library's design was to make it clearer when exceptions are thrown. So instead of methods like ThrowHelper.IfSelfNull() that work as little black boxes, the idea was to have the method do the validation logic, return true if it failed, and deliver the exception object by an out parameter, so that the calling code can do the actual throwing, ie if (Throw.ArgNull(nameof(arg), arg, out var exception)) { throw exception; }.

Those sharper than past-me probably already see the issue. Variable declared inline pollute the scope outside the if statements block, so if there are two args that both need to be checked, then they can't both use the name "exception" for their out parameters. Technically one could reuse the initial exception variable, but that is situational at best and misses the point.

// The second check will not compile as the exception variable is already defined.
if (Throw.ArgNull(nameof(arg1), arg1, out var exception)) { throw exception; }
// Compiler Error CS0128:
// A local variable named 'exception' is already defined in this scope
if (Throw.ArgNull(nameof(arg2), arg2, out var exception)) { throw exception; }

Quick Fix

There is a workaround, though at first impression it strikes me as dirty. Wrap each if-throw construct in its own scope block.

// This will compile as expected.
{ if (Throw.ArgNull(nameof(arg1), arg1, out var exception)) { throw exception; } }
{ if (Throw.ArgNull(nameof(arg2), arg2, out var exception)) { throw exception; } }

I have to say that I don't completely hate it. Some aspect of marking argument validation with a different "syntax" (for lack of a better term) strikes me as not terrible. That said, it definitely isn't ideal.

Solution

There isn't one. Either we stick with the current design and promote the workaround to normal practice, or we abandon the design completely and return something more standard. The former obviously isn't desirable, otherwise I'd be writing this from that perspective. The latter is a hard choice, but the breaking out of message formats, Throw.Formats, and the building of exceptions, Throw.Build, has brought most of the benefits sought when this library was initially designed.

So...

Plan of Action

  1. Deprecate the Throw methods as warnings.
  • Switch to deprecated as errors once internal early adopters have upgraded.
  • Remove the methods a couple pre-release versions after that.
  1. Implement more traditional Throw.If*() methods.
  2. Implement tests for the new methods.
Edited by Jay Jeckel