Skip to content
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

Merged
merged 39 commits into from
Aug 30, 2024

Conversation

davidepalladino-apuliasoft
Copy link
Collaborator

@davidepalladino-apuliasoft davidepalladino-apuliasoft commented Aug 28, 2024

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:

Value * DS Field Type Precision (Scale) Result Notes
'00520' ZONED 5 (0) 520
'00520' ZONED 5 (2) 5.2
'0052 ' ZONED Error The value must contain only numbers.
'00520' PACKED Error The value must not contain only numbers.

* 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 a NumberType, as ZONED, has the right syntax;
  • coercing file, that checks if the value is number and the destination RPG type is PACKED, to launch an Exception.
    Then, movel simply creates an instance of DataStructValue by reusing the same logic for EVAL, allowed for MOVE/MOVEL.

Related to #LS24003807

Checklist:

  • If this feature involves RPGLE fixes or improvements, they are well-described in the summary.
  • There are tests for this feature.
  • RPGLE code used for tests is easily understandable and includes comments that clarify the purpose of this feature.
  • The code follows Kotlin conventions (run ./gradlew ktlintCheck).
  • The code passes all tests (run ./gradlew check).
  • Relevant documentation is included in the docs directory.

…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.
@davidepalladino-apuliasoft davidepalladino-apuliasoft marked this pull request as draft August 28, 2024 14:04
@davidepalladino-apuliasoft
Copy link
Collaborator Author

Updated description of this PR.

@davidepalladino-apuliasoft davidepalladino-apuliasoft marked this pull request as ready for review August 29, 2024 11:15
@davidepalladino-apuliasoft davidepalladino-apuliasoft changed the title Bugfix/ls24003807/s to ds Bugfix/ls24003807/S to DS with positive number Aug 29, 2024
Copy link
Collaborator

@lanarimarco lanarimarco left a 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.

@lanarimarco lanarimarco merged commit fb9f97f into develop Aug 30, 2024
1 check passed
@lanarimarco lanarimarco deleted the bugfix/LS24003807/s-to-ds branch August 30, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants