1
\$\begingroup\$

I have an RPG program which processes a file containing records of subscription details of users. Each subscription record has a "valid from date" and "valid to date" - indicating when the subscription started and ended. I need to check if a subscription is currently valid (curdate holds current date) or should have been valid during jan-2021.

if curDate >= ValFrDate and curDate <= ValToDate                                                                                                                                                
  or                                                                                                                                                                                                                            
  %char(ValFrDate) <= '2021-31-01' and %char(ValToDate) >= '2021-01-01';   
   

The ValFrDate and ValToDate fields are defined as DATE datatype. Hence I am comparing it by casting to char first. Is there any better way to achieve this , perhaps without this casting? Also, I am wondering if the code can be made a bit more clearer as to what it is trying to achieve. Any suggestions please?

\$\endgroup\$
3
  • \$\begingroup\$ What is the significance of '2021-31-01'? Is that intended to be '2021-01-31'? If you really do have dates in middle-endian order like that, your comparison is broken (since e.g. all dates in February will satisfy the constraint). \$\endgroup\$ Apr 4, 2023 at 13:53
  • \$\begingroup\$ That was a mistake. it should have been '2021-31-01' \$\endgroup\$
    – Theju112
    Apr 5, 2023 at 11:08
  • \$\begingroup\$ 😖 my bad again!! yes, it should have been '2021-01-31' \$\endgroup\$
    – Theju112
    Apr 5, 2023 at 12:33

2 Answers 2

2
\$\begingroup\$

As usual, clearly communicating with human colleagues is the challenging aspect, as communicating a technical idea to the machine is straightforward.

(I have never written or read RPG, and I lack a sandbox to run this code.)

There are two production deployment scenarios. Either you anticipate that maintenance engineers

  1. will be proficient in RPG, or
  2. they will typically use other languages and only occasionally interact with RPG

precedence

if curDate >= ValFrDate and curDate <= ValToDate                                                                                                                                                
  or                                                                                                                                                                                                                            
  %char(ValFrDate) <= '2021-31-01' and %char(ValToDate) >= '2021-01-01';

This works fine for scenario 1.

In scenario 2 a maintainer may need to consult the docs to verify that {AND, OR} have different precedence and group the way the indenting suggests. Consider putting "redundant" ( ) parens around each conjunct to make that very explicit.


math conventions

Consider saying it similar to how a math textbook would usually phrase it:

if ValFrDate <= curDate and curDate <= ValToDate                                                                                                                                                

This makes the meaning more visually apparent, as it's a closer approximation to how a math text or python engineer would typically chain it:

if ValFrDate <= curDate <= ValToDate                                                                                                                                                

half-open interval

Consider changing your API so the "to" date is exclusive:

if ValFrDate <= curDate < ValToDate                                                                                                                                                

Routinely using such a convention for dates and numeric quantities leads to code and APIs which are easier to reason about, as the intervals compose.

OTOH your current convention is perhaps motivated by SQL's inclusive BETWEEN operator. If that's an important part of many of your WHERE clauses, then leave it unchanged.


consistency

There seems to be an invariant that is implicitly assumed:

ValFrDate <= ValToDate

This or surrounding code should signal an error if that does not hold.


casting

It's unclear why you're reluctant to cast to %char. You are always free to cast a constant like '2021-01-01' to %date, just once, and repeatedly compare against that.


extract helper

We have a pair of "within range?" disjuncts. Consider creating a trivial IN_RANGE(date, start, end) helper predicate.


ISO-8601

The documentation suggests that

The default internal format for date variables is *ISO.

%char(ValFrDate) <= '2021-31-01'

I don't understand what's going on there, as there's no thirty-first month. If you managed to turn the default into YDM then you really ought to document that. Better, supply an explicit format.

Notice that a mixture of {YMD, YDM} formatted dates would not signal any errors when comparing them. If you choose to always convert %char to %date, prior to comparison, you might find it a useful technique for preventing accidental mismatches, since the cast can signal error.

I recommend expunging YDM format from this (or any other) codebase. If an instant in time is represented as text, stick to the convention of representing it in ISO-8601 form. It is unambiguous, readily parsed, and has a useful collation order. Remember that a DateYMD < DateYDM character comparison is not meaningful, so it can't produce the correct result. https://imgs.xkcd.com/comics/iso_8601.png

Unit testing can help to reveal difficulties like format mismatches.


This trivial code snippet appears to achieve some of its design goals, though there are aspects worth discussing.

The YDM vs YMD formatting convention needs to be worked out prior to deploying in production. At a minimum we need a comment explaining the situation (preferably a test which throws fatal error if it sees unexpected default date format), plus adequate unit tests. The "break out an IN_RANGE() helper" item would fit in perfectly with unit tests.

I would be happy to delegate maintenance tasks for this codebase to colleagues proficient in RPG.

\$\endgroup\$
5
  • \$\begingroup\$ I'm surprised you say "ship it" given that the YDM text comparison is almost certainly wrong! I'd ask for more demonstration (e.g. the unit tests) before that verdict. \$\endgroup\$ Apr 4, 2023 at 15:53
  • \$\begingroup\$ It's not clear to me that it's wrong. I suspect there is review context missing, and that the default format has been modified prior to invoking these couple of lines. But I called it out for inspection, it's worth at least a comment describing the mode we need to be in. \$\endgroup\$
    – J_H
    Apr 4, 2023 at 15:55
  • \$\begingroup\$ I find it hard to see how it could be correct, as if we're using YDM, we'll get the 1st of February (as '2021-01-02') included when we shouldn't. That's why I'd ask for more evidence before concluding the review. \$\endgroup\$ Apr 4, 2023 at 15:59
  • \$\begingroup\$ Granted, will do. \$\endgroup\$
    – J_H
    Apr 4, 2023 at 16:02
  • \$\begingroup\$ @J_H, Thank you! Yes the date value was indeed wrong!! This has been changed to 2021-01-31 \$\endgroup\$
    – Theju112
    Apr 5, 2023 at 11:09
1
\$\begingroup\$

Congratulations for asking this question in free format RPG! The only thing I would add to J_H's excellent answer is that you might want to use date constants rather than converting date values to character. The syntax for a date constant is d'2021-01-15' to mean January 15th, 2021. This of course will depend on what you have for ctl-opt DATFMT(). By default it is *ISO, but if you are using something else, the format of the date in the date constant must comply with that format.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.