nunit icon indicating copy to clipboard operation
nunit copied to clipboard

Refactor Resolve method defined in IResolveConstraint and implemented in misc classes should be made idempotent

Open OsirisTerje opened this issue 5 months ago • 1 comments

The Resolve method as it is now must only be called once, as it changes the state of the class. It should be refactored to be able to be called more than once with no sideeffects.

OsirisTerje avatar Jun 10 '25 09:06 OsirisTerje

I got curious as to why we added the exception thrown when the class is not resolvable. Turns out to have been introduced in the second commit in the present repository back in 2009: "Adding files missing from initial import" i.e. when we moved from SourceForge to Bazaar! Suffice it to say that this has always been part of our fluent implementation.

The exception is thrown based on the current state of the IsResolvable property, which is false in two different situations:

  1. When something is pending, e.g. if we have received one or more operators but have not yet seen the operand. I think this is the situation for which the exception was intended, since the message we give is "A partial expression may not be resolved." Calling Resolve in this case is likely to be an internal error of some kind and I think the exception needs to be returned.

  2. When the constraint has already been resolved, which is the situation here. It may be possible to ignore this situation and not throw an exception at all but return the already completed constraint or at least to throw with a different message.

CharliePoole avatar Jun 10 '25 11:06 CharliePoole