tuxji commented on code in PR #875:
URL: https://github.com/apache/daffodil/pull/875#discussion_r1024576599
##########
daffodil-cli/src/main/scala/org/apache/daffodil/Main.scala:
##########
@@ -929,7 +930,7 @@ object Main {
processor = processor.withValidationMode(validate)
- setupDebugOrTrace(processor.asInstanceOf[DataProcessor], conf)
+ val debugProcessor =
setupDebugOrTrace(processor.asInstanceOf[DataProcessor], conf)
Review Comment:
In this scope, processor is a mutable var so you may not need
`debugProcessor`. Perhaps line 933 can replace processor exactly like line 931
already replaces processor:
`processor = setupDebugOrTrace(processor.asInstanceOf[DataProcessor], conf)`
That change should work unless the compiler complains about types, and it
also makes me think the method name might look better if it was renamed to
`withDebugOrTrace`.
There's a region further down where processor is a val, so debugProcessor is
really needed there. Mike, Steve, would you prefer that both regions use a
mutable var processor or an immutable val processor and a val debugProcessor?
I think it's best practice to use val and careful variable naming to make it
clear a value is changing, but var is probably fine in a small lexical scope
especially when processor is replaced 2 times, not just 1 time.
--
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]
