bsloane1650 commented on a change in pull request #422:
URL: https://github.com/apache/incubator-daffodil/pull/422#discussion_r498423115
##########
File path:
daffodil-core/src/main/scala/org/apache/daffodil/grammar/GrammarTerm.scala
##########
@@ -51,7 +52,8 @@ import org.apache.daffodil.dsom.Term
abstract class Gram(contextArg: SchemaComponent)
extends OOLAGHostImpl(contextArg)
with BasicComponent
- with GramRuntime1Mixin {
+ with GramRuntime1Mixin
+ with GramRuntime2Mixin {
Review comment:
I think what we are looking for here is typeclasses. If the
GramRuntime1Mixin was a typeclass, we could define it in the Runtime1 package;
and provide an implementation in the Runtime1 package for all of the Gram
objects while leaving the Gram objects themselves in the Core package.
Unfourtuantly, typeclasses are not a first class citizen in Scala; but they
can be emulated using implicits. https://scalac.io/typeclasses-in-scala/
In the past, I have been dissapointed with this approach; but I believe that
was largely a result of the implications with respect to boxing; which should
not be an issue in this case.
An alternative approach would be to just turn parser() and generateCode()
from methods of Gram (through a mixing) into simple functions in
Runtime1/Runtime2 that take Gram as an arguement.
In either case, I would suggest that the way forward is that we implement
Runtime2 within the Runtime2 package without adding a mixin to the Gram
objects. Once that is done and we have a good feel for the method; we can
discuss if it is worth refactoring Runtime1 as well.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
