This is great. I think the refactor below is totally worth it. It's only a few lines of code, being factored out, but there are other reasons why it's worth it.
It makes it clear that the behavior is intended to be exactly the same between the two uses, whereas otherwise you need to have comments pointing to/from the other piece of code saying "keep these consistent", and I hate that because my IDE will jump to getAssertFailureMessage in the mixin for me, but the comments I have to navigate around myself with grep/search, and the two blocks of code can drift, and if by accident somebody fixes one and not the other, then I have to wonder if the two were intended to diverge, and the comments not fixed, or they were intended to be the same, but just not updated simultaneously. In general, I hate doing in comments what I can do by way of code constructs. A trait is a pretty easy way to share even just a single small method. I also think the method name getAssertFailureMessage() adds value to the code. Makes it more readable. [ Full content available at: https://github.com/apache/incubator-daffodil/pull/117 ] This message was relayed via gitbox.apache.org for [email protected]
