寻找更清晰的方式来编码这个RPG LE

Buck和Benny就如何改进RPG代码给出了很多很好的建议。

• 使用免费格式以提高可读性
• 使用较长的描述性名称,以澄清实际情况。(当你本可以一开始就给出一个明确的名字时,不要让别人不得不用decypher这个名字)
• 使用子过程而不是子例程。从子过程返回值使其成为用户定义的函数,甚至更好

一个程序应该执行一个想法。这个过程中的每一件事都应该与做那件事有关。这叫做 cohesion 。保持你的程序相当小和简单。多小啊。麻省理工学院人工智能实验室负责人Semour Papert对让年轻学生对计算机编程来做他们感兴趣的事情很感兴趣。当他问其中一人他们认为手术应该有多大时,他得到的回答是“咬了一口”。

您希望最大限度地减少过程之间不必要的依赖关系,这样一个领域的更改就不太可能在另一个领域引起问题。这叫做 coupling .

在你的循环中,注意你检查了多少个“A”或“C”的地方,你对“A”重复了一次相同的代码块,然后对“C”重复了一遍。你本可以使用IF。。或。。相反,这样您就不会重复代码块,这可能会导致将来的维护问题。这违反了“不要重复自己”的原则。你可以把自己想象成一个整洁的管家,说,每件事都有一个地方[代码行],每件事情都有它的位置。

现在谈一个小问题。在各地,我看到人们使用相同的键立即使用SETLL,然后使用READE。请改用CHAIN。在外壳下,CHAIN执行SETLL完成的逻辑,然后执行READE的逻辑。一些人认为,他们把READE作为SETLL成功的条件来节省时间。但实际情况是,对于每个I/O语句,编译器都会生成代码来准备调用I/O模块的参数,调用该模块来执行I/O函数,然后处理返回的参数。对于两个I/O语句,您将执行两次操作。让CHAIN操作为您处理它,它也有机会获得一些内部效率。另外,你的RPG代码现在简单了一点。从各个角度来看都更好。

做好准备。。。

您应该使用嵌入式SQL,而不是使用传统的I/O语句。原因太多了,我真的不想在这里写一篇完整的论文。现在就相信我。在学习这方面的投资确实有回报。

您将学习SELECT、DECLARE和OPEN CURSORS(类似于打开的数据路径),然后从光标中FETCH记录,甚至一次FETCH或INSERT多个记录。

现在,真正重要的事情

传统的RPG范式通常是经过循环,每次循环处理一条记录,通常一次一条记录地对其他文件执行额外的I/O。

SQL使您能够将记录作为一个集合处理在单个语句中、整个文件中或多个文件中。你的RPG代码可以减少和简化 戏剧性地 通过在一条SQL语句中运行整个文件,再加上其他两个文件。

CREATE TABLE QTEMP/PURGING AS
( SELECT c.customer, ...
    FROM Customers c
    LEFT EXCEPTION JOIN Orders o
            on c.customer = o.customer
    LEFT EXCEPTION JOIN Invoices i
            on c.customer = i.customer
    WHERE c.customer not in 
           (select s.customer
              from secondfile s
              where marketing in ('A','C')
                and eventdate > '2007-12-31'
           )
) with data;

DELETE FROM secondfile x
  WHERE x.customer in
          (select p.customer
             from purging p
          );

DELETE FROM thirdfile x
  WHERE x.customer in
          (select p.customer
             from purging p
          );

DELETE FROM Customers x
  WHERE x.customer in
          (select p.customer
             from purging p
          );

四项声明。这就是一切。没有循环。一旦你把第一句话说对了,剩下的就很简单了。

创建表。。。WITH DATA将SELECT的结果写入新表。我只是把它放进QTEMP,因为我不确定你是否想保留它。LEFT EXCEPTION JOIN表示使用左侧表中的行,而右侧表中没有与搜索条件匹配的行。这使您可以选择不在订单历史记录中且不在发票文件中的客户记录。生成包含要清除的客户列表的文件后,就可以使用该列表从Customer Master和其他两个文件中删除这些客户。

我会写一个函数(子过程)。局部变量使这样的代码更容易使用,因为您不会与主线中的变量发生冲突。我发现创建一个函数可以帮助我组织自己的想法,有了组织的想法,我就能写出更好的代码。”“更好”当然是主观的,但这里我的意思是更容易理解,最重要的是,更容易改变,而不会在这个过程中破坏其他东西。

变量名称。。。哦。使用更长的名字——有意义的名字。下次你不得不看这个的时候,ACFLAG会让你的生活变得更糟(也许一年后?七年后?)我更喜欢本尼的 do_purge -它说明了它的意图。这可能是一个指标变量,可以真正明确这是一个是/否的决策点,但这肯定更容易理解 if do_purge = 'Y' 比理解更重要 if acflag = 'N' 。消极的逻辑增加了问题。该子例程受到相同的神秘命名约定的影响。检查客户。检查一下是什么?正在实现的业务功能是什么?如果不能简单地用名字来描述,那就太复杂了——做的事情太多了。业务功能是否为“检查活跃客户”?以这种方式命名子例程(或者更好的是,以这种方式编写函数名)。你的主线可能会变成

if custIsInactive(customerID);
  exsr purge_customer;
endif;

评论。本尼把他必须做的事情做得很好。原始代码只有一个注释,而且几乎完全没有帮助。我们可以看到订单实体被找到了——这就是为什么 if not %eof() 方法我们还可以看到,我们将要写一张唱片。但没有解释 为什么? 这些行动是重要的、可取的和有用的。这件事对我帮助很大。先写下评论。不是伪代码!宇宙历史上最糟糕的评论是这样的:

// if X > 10, reset y
if x > 10;
  y = 0;
endif;

这个评论只是分散了人们对代码的注意力。留白会更好。简单的代码不需要注释。更复杂的代码总是受益于解释意图的注释。什么样的注释对这个代码有帮助?用英语解释为什么代码A和C很重要。也许是因为

// don't purge customers we've done business with
// exception: if we emailed or cold called them
//            and they didn't buy anything in the 
//            past 6 years, purge them.
if (bicotc = 'A' and bistpd >= 20070101) or
   (bicotc = 'C' and bistpd >= 20070101);
  do_purge = 'Y';
endif;

我意识到张贴的代码并不能做到这一点,但从玻璃的这一边我看不出你是否 预定的 这是它的书写方式,或者这是一个我们还没有绊倒的bug。意见应澄清 意图 。相信我,当下一个人进入这个代码时,他会很乐意阅读代码A和C的简单英语原因,以及为什么特定日期很重要。也许是合并或收购的日期,a和C项目来自旧部门。。。代码本身无法解释。

如果评论不受欢迎(在某些地方也是如此),至少要避免“神奇代码”和“神奇数字”。这个怎么样:

if (bicotc = OHIO_BIG_TICKET and bistpd >= MERGER_DATE) or
   (bicotc = OHIO_LITTLE_TICKET and bistpd >= MERGER_DATE);
  do_purge = 'Y';
endif;

最后,回到一次做一件事的概念上来。这个“检查客户”子程序显然不仅仅是“检查”,它是在向VRCSTKBI写入记录。根据对情况的描述,这对我来说就像是一个bug。基于setll/reate循环 出现 代码正在查看订单历史文件。此子例程将为前10个项目写入VRCSTKBI,然后第11个项目使客户没有资格进行清除。但VRCSTKBI中有该客户的记录。哇。很多时候,我们都想以效率的名义将多个I/O操作捆绑在一起。我在这里告诉你,经过35年的努力,我同意唐纳德·克努思的观点:

“我们应该忘记小效率,比如说97%的时间: 过早的优化是万恶之源。"

想想业务功能。

1. 客户是否有资格被清除?
2. 记录要清除的客户。
3. 从子文件中清除客户。

如果每个业务操作都有单独的函数,那么将来编写、理解测试、调试和修改会更容易。编写3个子过程,用好名字命名它们,然后在主线中部署它们:

if custIsInactive(customerID);
  record_purged_customerID(customerID);
  purge_customer(customerID);
endif;

函数要么返回信息(custIsInactive),要么提供服务(purge_customer)。在“提供服务”功能中不应该有太多的业务逻辑来做出决策,提供信息的功能也不应该实现服务。这并不是因为混合和匹配本质上是邪恶的,而是因为一段代码做的事情越多,就越难理解和维护。我们只能在活动内存中保留少量内容,因此将业务函数抽象为单个项(函数名)的能力对于制作健壮的程序是一个非常强大的帮助。

你只需要客户一栏,这就是你所要做的全部——去掉暗示你选择更多栏目的三个点。

通过添加distinct关键字使清除表具有唯一性,然后您可以使用三个异常联接,这三个联接将更具可读性,因为分析不必从异常联接转换为非联接。如果临时清除表具有客户主键,则删除操作肯定会更快。

我也会添加一个下拉表以防万一。

drop table qtemp/purging;

CREATE TABLE QTEMP/PURGING AS
( SELECT distinct c.customer
FROM Customers c
LEFT EXCEPTION JOIN Orders o
        on c.customer = o.customer
LEFT EXCEPTION JOIN Invoices i
        on c.customer = i.customer
left    exception join secondfile s on 
        c.customer = s.customer and
          marketing in ('A','C')
            and eventdate > '2007-12-31'
       )
) with data;

这就是你想要的吗?

/free
    if pugfil = 'Y' and agent# <> agent#_old;
       exsr chkcus_sr;
       if do_purge = 'Y';
          write trcmasrr;
          delete arcmasrr;
       endif;
    endif;

    begsr chkcus_sr;
       // Assume we will purge this customer.
       do_purge = 'Y';
       setll orhkey drcst1;
       reade orhkey drcst1;
       dow not %eof(drcst1);
          // If bicotc is A or C and the date is January 1, 2007 or later, do NOT purge and stop checking other records.
          if (bicotc = 'A' and bistpd >= 20070101) or
             (bicotc = 'C' and bistpd >= 20070101);
             // Make sure you change the flag to say NO - DON'T PURGE THIS CUSTOMER
             do_purge = 'N';
             leavesr;
          endif;
          write vrcstkrr;
          // Looks like you are doing more processing here but you don't show the code...
          reade orhkey drcst1;
       enddo;
    endsr;
 /end-free

或者,如果你想坚持固定格式:

c                   ifeq      pugfil = 'Y' and
c                             agent# <> agent#_old
c                   exsr      chkcus_sr
c                   if        do_purge = 'Y'
c                   write     trcmasrr
c                   delete    arcmasrr
c                   endif
 *------------------------------------------------------------------
c     chkcus_sr     begsr      
c                   eval      do_purge = 'Y'
c     orhkey        setll     drcst1
c     orhkey        reade     drcst1
 * If the order entity is found, write the rec into VRCSTKBI file
c                   dow       not %eof(drcst1)
c                   if        (bicotc = 'A' and bistpd >= 20070101) or
c                             (bicotc = 'C' and bistpd >= 20070101)
c                   eval      do_purge = 'N'
c                   leavesr
c                   endif

c                   write     vrcstkrr
 * // Do some other processing here that you don't show...
c     orhkey        reade     drcst1
c                   enddo