mbeckerle commented on PR #1058: URL: https://github.com/apache/daffodil/pull/1058#issuecomment-1646024096
> Is there much value from having them separate function names? With a goal of simplifying things, what if we changed them all to something like `Assert.assert(...)`. For both the argCheck and stateCheck cases, we are really just doing a bugCheck, so maybe a more generic name would be reasonable for both? Based on some of your comments there are places where usage should be invariant or vice versa so there's clearly some confusing between the different names that this would alleviate. Yeah, in hind-sight now I think almost all those changes of usage to invariant are wrong. Just because something isn't an arg doesn't mean it's not a usage error. > > Then the usage() function can be this "public API usage thing", or maybe a new name would be better to make it more clear that it's about incorrect public API usage. argCheck makes it clear that it's about checking arguments for proper usage. That is perhaps a useful new method. But precondition checks are another kind of usage-error check. We have lots of lazy vals in our code where there are no arguments, but there are still usage errors in that one can "use them wrong" and we want to make clear when you are using them wrong. If that kind of check is not going to be usage(...) they should be either argCheck(...) or precondition(...) both of which, when they fail, issue a kind of usage error. They indicate a programmer's fairly immediate local mistake in the way the thing was called or nearby context of that call. I prefer usage(...) myself, because in lazy code often things being checked are not arguments. invariant() is for things that are potentially more distant from the local calling context. Really its just about making assumptions of the code explicit so if under maintenance somebody changes the way things work and things code is relying on change, that there are explicit checks to detect this. It's also used for postcondition checks - when a certain method finishes some condition should hold. So I'm ok with argCheck, preCondition, and invariant as names of different methods. But usage an invariant are sufficient I think. We should improve the scaladoc on those sorts of methods, but I'm very opposed to changing the name of 'invariant' to anything else or losing the distinction of usage errors (arg or precondition) vs. invariant failure. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
