bsloane1650 commented on a change in pull request #422:
URL: https://github.com/apache/incubator-daffodil/pull/422#discussion_r498457789
##########
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:
This strikes me as a functional vs object-orientiented distinction.
In the functional view, the grammar objects are just a data structure. By
definition the "implementation" is in functions which are defined seperately.
It is then a purely organizational question if those functions should be
defined near where the data type is defined.
I don't think there is much glue logic to be dealt with.
Setting aside the possibility of typeclasses, all we are talking about is a
single parse() function with a match structure. This is a pattern we already
use throughout the compiler. At least to me, the functions defined with this
pattern are far easier to read, as all of the different versions of those
functions can be found in one place; instead of needing to go through all of
the different objects where different implementations of said functions are
located.
Needing to maintain the "parser" function when its implementation is
scattered throughout the code seems much more tedious than needing to update it
whenever a grammar object is added/removed (which happens far less frequently
than implementation changes).
With the addition of Runtime2; we are now putting parser logic, code
generation logic, and abstract grammar logic all in the same spot.
I agree that it may not be worth refactoring Runtime1. But we do not yet
have any intertia behind Runtime2; so this is the best time to change how we
implement it.
----------------------------------------------------------------
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]
