jadams-tresys commented on code in PR #1509:
URL: https://github.com/apache/daffodil/pull/1509#discussion_r2214094486
##########
daffodil-core/src/main/scala/org/apache/daffodil/runtime1/processors/RuntimeData.scala:
##########
@@ -357,59 +357,73 @@ final class SimpleTypeRuntimeData(
}
}
// Check length
- e.length.foreach { length =>
- if (!checkLength(currentElement, length, e, primType))
- return Error("facet length (%s)".format(length))
- }
+ val lengthError = e.length
+ .find(length => !checkLength(currentElement, length, e, primType))
+ .map(length => Error("facet length (%s)".format(length)))
+
+ if (lengthError.isDefined) return lengthError.get
+
Review Comment:
These feel clunky, but I'm not familiar enough with Scala 3 to know if
there's a better way or not.
##########
daffodil-core/src/main/scala/org/apache/daffodil/lib/calendar/DFDLCalendar.scala:
##########
@@ -134,25 +134,39 @@ trait OrderedCalendar { self: DFDLCalendar =>
Assert.invariant(!p.hasTimeZone || p.calendar.getTimeZone ==
TimeZone.GMT_ZONE)
Assert.invariant(!q.hasTimeZone || q.calendar.getTimeZone ==
TimeZone.GMT_ZONE)
+ var result: DFDLCalendarOrder = DFDLCalendarOrder.P_EQUAL_Q
+ var i = 0
val length = fieldsForComparison.length
- for (i <- 0 until length) {
+ var done = false
+
+ while (i < length && !done) {
Review Comment:
Note that I'm not sure this will work when the field isn't set for a
calendar. Was unable to test that at the moment.
##########
daffodil-core/src/main/scala/org/apache/daffodil/lib/calendar/DFDLCalendar.scala:
##########
@@ -134,25 +134,39 @@ trait OrderedCalendar { self: DFDLCalendar =>
Assert.invariant(!p.hasTimeZone || p.calendar.getTimeZone ==
TimeZone.GMT_ZONE)
Assert.invariant(!q.hasTimeZone || q.calendar.getTimeZone ==
TimeZone.GMT_ZONE)
+ var result: DFDLCalendarOrder = DFDLCalendarOrder.P_EQUAL_Q
+ var i = 0
val length = fieldsForComparison.length
- for (i <- 0 until length) {
+ var done = false
+
+ while (i < length && !done) {
Review Comment:
I think this whole function and while loop could probably be converted into
a collectFirst statement as well, something like:
```
val diffField = fieldsForComparison.find { field =>
p.calendar.get(field) != h.calendar.get(field)
}
res = if (diffField.isDefined) {
... compare this field on both calendars and return the appropriate
DFDLCalendarOrder...
}
else DFDLCalendarOrder.P_EQUAL_Q
return res
```
##########
daffodil-core/src/test/scala/org/apache/daffodil/core/dsom/TestMiddleEndAttributes.scala:
##########
@@ -179,7 +179,10 @@ class TestMiddleEndAttributes {
val mems = seq.groupMembers
val Seq(t1: Term) = mems
val e1ref = t1.asInstanceOf[ElementRef]
- val Some(nes: LocalSequence) = e1ref.optLexicalParent
+ val nes = e1ref.optLexicalParent match {
+ case Some(nes: LocalSequence) => nes
+ case _ => fail(); null
+ }
Review Comment:
These sorts of assignments are all over the place now. Was this a
recommended way of doing things in Scala 3? Also seems rather clunky.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/PropProviders.scala:
##########
@@ -141,17 +141,24 @@ final class ChainPropProvider(leafProvidersArg:
Seq[LeafPropProvider], forAnnota
sources: Seq[LeafPropProvider],
pname: String
): PropertyLookupResult = {
- val allNotFound =
- for { source <- sources } yield {
- val res = source.leafFindProperty(pname)
- res match {
- case _: Found => return res // found it! return right now.
- case nf @ NotFound(_, _, _) => nf
- }
+ var result: Option[PropertyLookupResult] = None
+ val allNotFound = scala.collection.mutable.ListBuffer.empty[NotFound]
+
+ val iter = sources.iterator
+ while (iter.hasNext && result.isEmpty) {
+ val res = iter.next().leafFindProperty(pname)
+ res match {
+ case f: Found =>
+ result = Some(f)
+ case nf: NotFound =>
+ allNotFound += nf
Review Comment:
This doesn't feel very Scala like to me. Couldn't we do something like
```
val res = sources.collectFirst{ source =>
source.leafFindProperty(pname) match {
case f: Found => Some(f)
}
}
res.getOrElse(...)
```
--
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]
