-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix/ls24003807/S to DS with positive number #600
Conversation
…sh beacuse for fields 3 and 4 are passed Standard numbers
…sh beacuse for fields 3 and 4 are passed Standard numbers
…sh because for fields 3 and 4 are passed strings
…h because for fields 3 and 4 are passed strings
… duplication and to improve the logic of coercion from String to DS in EVAL.
Updated description of this PR. |
rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/coercing.kt
Outdated
Show resolved
Hide resolved
rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/coercing.kt
Outdated
Show resolved
Hide resolved
rpgJavaInterpreter-core/src/main/kotlin/com/smeup/rpgparser/interpreter/values.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data structure implementation is one of the most difficult (or worst) parts of Jariko.
This PR passes all tests, but frankly, regarding the change to the fun get(data: FieldDefinition): Value
operator, I cannot confidently predict if this will cause regressions in real-world scenarios.
What concerns me is that we're allowing the creation of an incorrect data structure. There’s no validation in the constructor or setter methods, with validation only present in the fun get(data: FieldDefinition): Value
. However, I don’t want to spend more time on this code review because it may not be productive, and then let's move on.
Description
This work resolves the issue when using the
MOVE
/MOVEL
operations to move an alphanumeric standalone field into a data structure field. The table below describes the currently handled use cases:'00520'
ZONED
'00520'
ZONED
'0052 '
ZONED
'00520'
PACKED
* standalone field value or sub-value
Techincal notes
Every sub-value have to match with related
DS
field. To fix these problems in Jariko, I improved:DataStructValue
, that checks if aNumberType
, asZONED
, has the right syntax;coercing
file, that checks if the value is number and the destination RPG type isPACKED
, to launch an Exception.Then,
movel
simply creates an instance ofDataStructValue
by reusing the same logic forEVAL
, allowed forMOVE
/MOVEL
.Related to #LS24003807
Checklist:
./gradlew ktlintCheck
)../gradlew check
).docs
directory.