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

Feature/ls24003884/Evaluation blank value of DS field #603

Merged
merged 10 commits into from
Sep 3, 2024

Conversation

davidepalladino-apuliasoft
Copy link
Collaborator

@davidepalladino-apuliasoft davidepalladino-apuliasoft commented Sep 3, 2024

Description

This work resolves evaluation at runtime of a DS' field with only blank values.

Technical notes

This is a regression issue of checkNumberSyntax. To resolve this problem I put a checking if the value is completely blank. In this case passes and coerce to right value.

Related to:

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.

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.

There’s a better way to reference the cause of the regression. Since we work with pull requests, you can simply include #<prnumber> in your PR description. This will automatically create a link to the related pull request.

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.

  • Since checkNumberSyntax does not access any instance properties of DataStructValue, it should be declared inside the companion object.
  • Make it private or internal (if you want test it).
  • According to the comment, a ZONED number with at least one space at the end is not allowed if it contains a number. However, this suggests that if the value is "abc" and the type is ZONED, it would be accepted. Is this correct?

@davidepalladino-apuliasoft
Copy link
Collaborator Author

  • Since checkNumberSyntax does not access any instance properties of DataStructValue, it should be declared inside the companion object.

  • Make it private or internal (if you want test it).

Done!

  • According to the comment, a ZONED number with at least one space at the end is not allowed if it contains a number. However, this suggests that if the value is "abc" and the type is ZONED, it would be accepted. Is this correct?

I tested this code on AS400:

     D A40_A50         S             50
     D A40_DS1         DS
     D  A40_DS1_F1             1     20
     D  A40_DS1_F2            21     40
     D  A40_DS1_F3            41     45  0
     D  A40_DS1_F4            46     50S 2

     D A40_DS1_F4_S    S             20

     C                   EVAL      A40_A50 = 'Lorem ipsum dolor si'
     C                                         + 't amet, consectetuer'
     C                                         + 'abcde00520'

     C                   EVAL      A40_DS1=A40_A50
     C     A40_DS1_F1    DSPLY
     C     A40_DS1_F2    DSPLY
     C     A40_DS1_F3    DSPLY
     C                   EVAL      A40_DS1_F4_S=%CHAR(A40_DS1_F4)
     C     A40_DS1_F4_S  DSPLY

and the result is:
immagine

Do you want an implementation of this case for this PR?

@lanarimarco
Copy link
Collaborator

  • Since checkNumberSyntax does not access any instance properties of DataStructValue, it should be declared inside the companion object.
  • Make it private or internal (if you want test it).

Done!

  • According to the comment, a ZONED number with at least one space at the end is not allowed if it contains a number. However, this suggests that if the value is "abc" and the type is ZONED, it would be accepted. Is this correct?

I tested this code on AS400:

     D A40_A50         S             50
     D A40_DS1         DS
     D  A40_DS1_F1             1     20
     D  A40_DS1_F2            21     40
     D  A40_DS1_F3            41     45  0
     D  A40_DS1_F4            46     50S 2

     D A40_DS1_F4_S    S             20

     C                   EVAL      A40_A50 = 'Lorem ipsum dolor si'
     C                                         + 't amet, consectetuer'
     C                                         + 'abcde00520'

     C                   EVAL      A40_DS1=A40_A50
     C     A40_DS1_F1    DSPLY
     C     A40_DS1_F2    DSPLY
     C     A40_DS1_F3    DSPLY
     C                   EVAL      A40_DS1_F4_S=%CHAR(A40_DS1_F4)
     C     A40_DS1_F4_S  DSPLY

and the result is: immagine

Do you want an implementation of this case for this PR?

In my opinion checkNumber for ZONED type should be accept only blank or only numbers without blank

@davidepalladino-apuliasoft
Copy link
Collaborator Author

  • Since checkNumberSyntax does not access any instance properties of DataStructValue, it should be declared inside the companion object.
  • Make it private or internal (if you want test it).

Done!

  • According to the comment, a ZONED number with at least one space at the end is not allowed if it contains a number. However, this suggests that if the value is "abc" and the type is ZONED, it would be accepted. Is this correct?

I tested this code on AS400:

     D A40_A50         S             50
     D A40_DS1         DS
     D  A40_DS1_F1             1     20
     D  A40_DS1_F2            21     40
     D  A40_DS1_F3            41     45  0
     D  A40_DS1_F4            46     50S 2

     D A40_DS1_F4_S    S             20

     C                   EVAL      A40_A50 = 'Lorem ipsum dolor si'
     C                                         + 't amet, consectetuer'
     C                                         + 'abcde00520'

     C                   EVAL      A40_DS1=A40_A50
     C     A40_DS1_F1    DSPLY
     C     A40_DS1_F2    DSPLY
     C     A40_DS1_F3    DSPLY
     C                   EVAL      A40_DS1_F4_S=%CHAR(A40_DS1_F4)
     C     A40_DS1_F4_S  DSPLY

and the result is: immagine
Do you want an implementation of this case for this PR?

In my opinion checkNumber for ZONED type should be accept only blank or only numbers without blank

I can provide to improve checkNumberSyntax with you case in addition to abc45 and 12cde cases.

@lanarimarco lanarimarco merged commit e41b169 into develop Sep 3, 2024
1 check passed
@lanarimarco lanarimarco deleted the feature/LS24003884/blank-value-to-ds branch September 3, 2024 13:38
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