9
\$\begingroup\$

I was long a professional in COBOL, but noone ever dared comment my style. As the FizzBuzz seems fashion here, I made one. Verbose, of course, we're speaking about COBOL. I know I could have made in 15 lines, but decided instead to provide the full structure for a production program. Result makes 87 lines.

I'll be happy to read your thoughts, comments, critics, or executions. Made on OpenCOBOLIDE.

 ****************************************************************** * Author:Gazzz0X2Z * * Date:2015, Dec the 12th * * Purpose:FizzBuzz, what else? * * Tectonics: cobc * ****************************************************************** IDENTIFICATION DIVISION. *-*-*-*-*-*-*-*-*-*-*-*-*- PROGRAM-ID. FIZZBUZZ. *-*-*-*-*-*-*-*-*-*-*-- ENVIRONMENT DIVISION. *-*-*-*-*-*-*-*-*-*-*-- CONFIGURATION SECTION. *---------------------- INPUT-OUTPUT SECTION. *---------------------- DATA DIVISION. *-*-*-*-*-*-*-*- FILE SECTION. *------------------------- WORKING-STORAGE SECTION. *------------------------- 01 WS-DATA. 05 WS-COUNTS. 10 WS-FIRST PIC 9(4) VALUE ZERO. 10 WS-LAST PIC 9(4) VALUE 20. 10 WS-POSN PIC 9(4). 05 WS-RESULTS. 10 WS-RS00 PIC 9(4). 10 WS-RM05 PIC 9(4). 88 WS-ML05 VALUE ZERO. 10 WS-RM03 PIC 9(4). 88 WS-ML03 VALUE ZERO. 05 WS-DISPLAY-STRING PIC X(10). 05 WS-DISPLAY-NUM REDEFINES WS-DISPLAY-STRING. 10 WS-FIZZBUZZ-PRI PIC X(3). 10 WS-FIZZBUZZ-INT PIC Z(3)9. 10 WS-FIZZBUZZ-SUI PIC X(3). 05 WS-DISPLAY-ALPHA REDEFINES WS-DISPLAY-STRING. 10 WS-FIZZBUZZ-STR PIC X(10). 88 WS-IS-OTHER VALUE "* 0000 *". 88 WS-IS-FIZZ VALUE "* FIZZ *". 88 WS-IS-BUZZ VALUE "* BUZZ *". 88 WS-IS-FIZZBUZZ VALUE "*FIZZBUZZ*". *-*-*-*-*-*-*-*-*-*-* PROCEDURE DIVISION. *-*-*-*-*-*-*-*-*-*-* MAIN-PROCEDURE. PERFORM 10000-BEGIN PERFORM 20000-MAIN VARYING WS-POSN FROM WS-FIRST BY 1 UNTIL WS-POSN > WS-LAST PERFORM 30000-END . STOP RUN. * 10000-BEGIN. DISPLAY "**********" . * 20000-MAIN. PERFORM 40000-COMPUTE-FIZZBUZZ DISPLAY WS-DISPLAY-STRING . * 30000-END. DISPLAY "**********" . * 40000-COMPUTE-FIZZBUZZ. DIVIDE WS-POSN BY 5 GIVING WS-RS00 REMAINDER WS-RM05 DIVIDE WS-POSN BY 3 GIVING WS-RS00 REMAINDER WS-RM03 EVALUATE TRUE WHEN (WS-ML05 AND WS-ML03) SET WS-IS-FIZZBUZZ TO TRUE WHEN WS-ML05 SET WS-IS-BUZZ TO TRUE WHEN WS-ML03 SET WS-IS-FIZZ TO TRUE WHEN OTHER SET WS-IS-OTHER TO TRUE MOVE WS-POSN TO WS-FIZZBUZZ-INT END-EVALUATE . * END PROGRAM FIZZBUZZ.

Result : 

********** *FIZZBUZZ* * 1 * * 2 * * FIZZ * * 4 * * BUZZ * * FIZZ * * 7 * * 8 * * FIZZ * * BUZZ * * 11 * * FIZZ * * 13 * * 14 * *FIZZBUZZ* * 16 * * 17 * * FIZZ * * 19 * * BUZZ * **********
\$\endgroup\$

5 Answers 5

Reset to default
4
\$\begingroup\$

As I'm sure you are aware, COBOL tends to be written one way at one site, another way at another.

Often things like name prefixes, and paragraph/SECTION prefixes cannot be other than dictated. Same with case of identifiers.

I dislike WSnn- (drives people nuts trying to insert numbers so that new data can be located rationally rather than in the order of arrival in specification) and WS- (similar for the LINKAGE SECTION) but am happy with W-/L-/LS- (or nothing). Absolutely hate single-word identifiers. Even these days with forewarning of what may appear later, it is plain dumb to have them there waiting for some compiler-change to make them invalid.

Also, since COBOL has no concept of "main", it is something to avoid pretending exists.

For paragraphs, or SECTIONS, I go with two numerics. Then with each subsequent level, append an alpha, A, D, G, J, M etc (allowing for insertions), and keep all related paragraphs/SECTIONS together, physically. Your program only has one level, so it doesn't really matter what you do in that one.

Nicely laid-out. I tend to be more extreme, but the effort you've invested impresses the reader (if they are a programmer, rather than a 9-5-er).

Hate the comments.

I'd remove all the redundant code (you don't need an ENVIRONMENT DIVISION, for instance).

Ah, for these last two, they have been generated for you, so I'd suggest copying an existing model program rather than using the OpenCOBOLIDE bolierplate.

I only use VARYING with things that, well, vary. So I'd use TIMES. Describe the situation better. You have to do more typing, but in addition to the better description of the data, your code will be faster than using VARYING.

Forget ideas like using REM. COBOL has done remainders since day one, a considerable time ago, so why (I don't know the answer to this) is there a function for it? In Enterprise COBOL (IBM Mainframe) REM would return a floating-point item, then converted to what you define in your program. Wow, do we need that?

I would work on the data-names a bit, but they are better than VAR1, VAR2 (or WS13-VAR1 and WS34-VAR2).

Here's an example of how I like to format a program:

http://ibmmainframes.com/viewtopic.php?p=278927#278927

And since that is a link, here's the code:

 IDENTIFICATION DIVISION. PROGRAM-ID. BBHEXD. ENVIRONMENT DIVISION. DATA DIVISION. WORKING-STORAGE SECTION. 01 W-WHEN-COMPILED PIC X(8)BX(8). 01 W-DISPLAY-HEX-MAX-UIND USAGE IS INDEX. 01 W-FIRST-TIME-FLAG PIC X VALUE "Y". 88 W-FIRST-TIME-IN-PROGRAM VALUE "Y". 88 W-NOT-FIRST-TIME-FLAG VALUE "N". 01 W-WORK-BYTE-FOR-CALC COMP PIC S9(4) VALUE ZERO. 01 FILLER REDEFINES W-WORK-BYTE-FOR-CALC. 05 FILLER PIC X. 05 W-WORK-BYTE PIC X. 01 W-LEFT-HALF-BYTE-NUM COMP PIC S9(4). 01 W-RIGHT-HALF-BYTE-NUM COMP PIC S9(4). 01 W-BYTES-TO-OUTPUT COMP PIC S9(4). 01 W-BYTES-MOVED COMP PIC S9(4). 01 W-HEX-ALPHA-SUB COMP PIC S9(4). 01 W-HEX-ALPHA-TABLE VALUE "ABCDEF". 05 FILLER OCCURS 6 TIMES INDEXED BY W-HEX-ALPHA-IDX. 10 W-HEX-ALPHA PIC X. 01 W-DISPLAY-HEX. 05 W-LEFT-HEX PIC X. 05 W-RIGHT-HEX PIC X. 01 W-HEX-TO-DISPLAY-LEFT-TABLE VALUE SPACE. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-BYTES-TO-OUTPUT INDEXED BY W-DISPLAY-HEX-LEFT-IND. 10 W-HEX-TO-DISPLAY-LEFT PIC X. 88 W-HEX-TO-DISPLAY-LEFT-IS-EMPTY VALUE SPACE. 01 W-HEX-TO-DISPLAY-RIGHT-TABLE VALUE SPACE. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-BYTES-TO-OUTPUT INDEXED BY W-DISPLAY-HEX-RIGHT-IND. 10 W-HEX-TO-DISPLAY-RIGHT PIC X. 01 W-BYTE-TO-DISPLAY-TABLE VALUE SPACE. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-BYTES-TO-OUTPUT INDEXED BY W-DISPLAY-BYTE-IND. 10 W-BYTE-TO-DISPLAY PIC X. 01 W-CONVERT-TO-HEX-FROM-DIGIT COMP PIC S9(4). 01 W-CONVERTED-HEX-DIGIT PIC X. 01 W-CONVERTED-HEX-DIGIT-NUM REDEFINES W-CONVERTED-HEX-DIGIT PIC 9. 01 W-CONVERTED-HEX-DIGIT-NUM4 PIC 9(4). 01 FILLER REDEFINES W-CONVERTED-HEX-DIGIT-NUM4. 05 FILLER PIC XXX. 05 W-CONVERTED-HEX-DIGIT-NUM4L PIC 9. 01 W-COLS-LINE. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-BYTES-TO-OUTPUT. 10 FILLER PIC X. 01 W-COLS-TABLE. 05 FILLER PIC X(10) VALUE "----+----1". 05 FILLER PIC X(10) VALUE "----+----2". 05 FILLER PIC X(10) VALUE "----+----3". 05 FILLER PIC X(10) VALUE "----+----4". 05 FILLER PIC X(10) VALUE "----+----5". 05 FILLER PIC X(10) VALUE "----+----6". 05 FILLER PIC X(10) VALUE "----+----7". 05 FILLER PIC X(10) VALUE "----+----8". 05 FILLER PIC X(10) VALUE "----+----9". 05 FILLER PIC X(10) VALUE "----+----0". 05 FILLER PIC X(10) VALUE "----+----1". 05 FILLER PIC X(10) VALUE "----+----2". 05 FILLER PIC X(10) VALUE "----+----3". 05 FILLER PIC X(02) VALUE "--". LINKAGE SECTION. 01 L-DATA-LENGTH COMP PIC S9(8). 01 L-BBHEXD-CONTROL-BLOCK. 05 L-BBHEXD-CB-MAX-LINE COMP PIC S9(4) VALUE +80. 88 L-BBHEXD-CB-MAX-LINE-VALID VALUE +80 +132. 05 L-BBHEXD-CB-CHUNK-LENGTH COMP PIC S9(4) VALUE ZERO. 88 L-BBHEXD-CB-CHUNK-LENGTH-ZERO VALUE ZERO. 05 FILLER PIC X(96) VALUE SPACE. 01 L-DATA-PICX1-FOR-USING PIC X. 01 L-DATA-TO-DISPLAY REDEFINES L-DATA-PICX1-FOR-USING. 05 FILLER OCCURS 99999999 TIMES INDEXED BY L-DTD-BYTE-FOR-C-AND-C-IDX. 10 L-DTD-BYTE-FOR-CONV-AND-COPY PIC X. PROCEDURE DIVISION USING L-DATA-PICX1-FOR-USING L-DATA-LENGTH L-BBHEXD-CONTROL-BLOCK . 00-CONTROL SECTION. PERFORM 10-FIRST-TIME PERFORM 30-CNV-LK-DATA-TO-HEX-AND-DISP GOBACK . 10-FIRST-TIME SECTION. IF W-FIRST-TIME-IN-PROGRAM PERFORM 10A-SAY-WHO-WE-ARE SET W-NOT-FIRST-TIME-FLAG TO TRUE END-IF PERFORM 10D-CHECK-OUTPUT-LENGTH PERFORM 10G-SET-OUTPUT-LNGTH-AND-CHUNK . 10A-SAY-WHO-WE-ARE SECTION. MOVE WHEN-COMPILED TO W-WHEN-COMPILED DISPLAY "BBHEXD This program compiled on " W-WHEN-COMPILED . 10D-CHECK-OUTPUT-LENGTH SECTION. IF ( NOT L-BBHEXD-CB-MAX-LINE-VALID ) DISPLAY "BBHEXD maximum line length should be 80 or 132" DISPLAY "BBHEXD found>" L-BBHEXD-CB-MAX-LINE "<" CALL "FOODUMP" END-IF . 10G-SET-OUTPUT-LNGTH-AND-CHUNK SECTION. IF L-BBHEXD-CB-CHUNK-LENGTH-ZERO MOVE L-BBHEXD-CB-MAX-LINE TO W-BYTES-TO-OUTPUT ELSE IF ( L-BBHEXD-CB-CHUNK-LENGTH GREATER THAN L-BBHEXD-CB-MAX-LINE ) DISPLAY "BBHEXD Sorry, chunks ignored, too big" DISPLAY "BBHEXD Chunks>" L-BBHEXD-CB-CHUNK-LENGTH "<" DISPLAY "BBHEXD Maxlen>" L-BBHEXD-CB-MAX-LINE "<" MOVE L-BBHEXD-CB-MAX-LINE TO W-BYTES-TO-OUTPUT ELSE MOVE L-BBHEXD-CB-CHUNK-LENGTH TO W-BYTES-TO-OUTPUT END-IF END-IF SET W-DISPLAY-BYTE-IND TO W-BYTES-TO-OUTPUT SET W-DISPLAY-HEX-MAX-UIND TO W-DISPLAY-BYTE-IND IF L-DATA-LENGTH EQUAL TO ZERO DISPLAY "BBHEXD You want to display zero bytes?" DISPLAY "BBHEXD Do not expect to see them" END-IF . 30-CNV-LK-DATA-TO-HEX-AND-DISP SECTION. SET W-DISPLAY-HEX-LEFT-IND W-DISPLAY-HEX-RIGHT-IND W-DISPLAY-BYTE-IND TO +1 MOVE ZERO TO W-BYTES-MOVED PERFORM 30A-BYTE-BY-BYTE-CONVERT VARYING L-DTD-BYTE-FOR-C-AND-C-IDX FROM 1 BY 1 UNTIL L-DTD-BYTE-FOR-C-AND-C-IDX GREATER THAN L-DATA-LENGTH IF ( W-BYTES-MOVED NOT EQUAL TO ZERO ) MOVE W-BYTES-MOVED TO W-BYTES-TO-OUTPUT PERFORM 99A-DISPLAY-PART-OF-OUTPUT END-IF . 30A-BYTE-BY-BYTE-CONVERT SECTION. MOVE L-DTD-BYTE-FOR-CONV-AND-COPY ( L-DTD-BYTE-FOR-C-AND-C-IDX ) TO W-WORK-BYTE DIVIDE W-WORK-BYTE-FOR-CALC BY 16 GIVING W-LEFT-HALF-BYTE-NUM REMAINDER W-RIGHT-HALF-BYTE-NUM MOVE W-LEFT-HALF-BYTE-NUM TO W-CONVERT-TO-HEX-FROM-DIGIT PERFORM 30AA-CONVERT-HALF-BYTE MOVE W-CONVERTED-HEX-DIGIT TO W-LEFT-HEX MOVE W-RIGHT-HALF-BYTE-NUM TO W-CONVERT-TO-HEX-FROM-DIGIT PERFORM 30AA-CONVERT-HALF-BYTE MOVE W-CONVERTED-HEX-DIGIT TO W-RIGHT-HEX IF ( W-DISPLAY-HEX-LEFT-IND GREATER THAN W-DISPLAY-HEX-MAX-UIND ) PERFORM 99A-DISPLAY-PART-OF-OUTPUT SET W-DISPLAY-HEX-LEFT-IND W-DISPLAY-HEX-RIGHT-IND W-DISPLAY-BYTE-IND TO +1 END-IF MOVE W-LEFT-HEX TO W-HEX-TO-DISPLAY-LEFT ( W-DISPLAY-HEX-LEFT-IND ) MOVE W-RIGHT-HEX TO W-HEX-TO-DISPLAY-RIGHT ( W-DISPLAY-HEX-RIGHT-IND ) MOVE W-WORK-BYTE TO W-BYTE-TO-DISPLAY ( W-DISPLAY-BYTE-IND ) ADD +1 TO W-BYTES-MOVED SET W-DISPLAY-HEX-LEFT-IND W-DISPLAY-HEX-RIGHT-IND W-DISPLAY-BYTE-IND UP BY +1 . 30AA-CONVERT-HALF-BYTE SECTION. IF ( W-CONVERT-TO-HEX-FROM-DIGIT GREATER THAN 9 ) SUBTRACT 9 FROM W-CONVERT-TO-HEX-FROM-DIGIT GIVING W-HEX-ALPHA-SUB SET W-HEX-ALPHA-IDX TO W-HEX-ALPHA-SUB MOVE W-HEX-ALPHA ( W-HEX-ALPHA-IDX ) TO W-CONVERTED-HEX-DIGIT ELSE MOVE W-CONVERT-TO-HEX-FROM-DIGIT TO W-CONVERTED-HEX-DIGIT-NUM4 MOVE W-CONVERTED-HEX-DIGIT-NUM4L TO W-CONVERTED-HEX-DIGIT-NUM END-IF . 99A-DISPLAY-PART-OF-OUTPUT SECTION. MOVE W-COLS-TABLE TO W-COLS-LINE DISPLAY W-COLS-LINE DISPLAY W-BYTE-TO-DISPLAY-TABLE DISPLAY W-HEX-TO-DISPLAY-LEFT-TABLE DISPLAY W-HEX-TO-DISPLAY-RIGHT-TABLE MOVE SPACE TO W-BYTE-TO-DISPLAY-TABLE W-HEX-TO-DISPLAY-LEFT-TABLE W-HEX-TO-DISPLAY-RIGHT-TABLE MOVE ZERO TO W-BYTES-MOVED .

To demonstrate the usefulness of meaningful names, here's a version with the names rubbished:

 IDENTIFICATION DIVISION. PROGRAM-ID. UGLY. ENVIRONMENT DIVISION. DATA DIVISION. WORKING-STORAGE SECTION. 01 W-A PIC X(8)BX(8). 01 W-B USAGE IS INDEX. 01 W-C USAGE IS INDEX. 01 W-D PIC S9(9). 01 FILLER REDEFINES W-D. 05 FILLER PIC X(5). 05 W-E PIC 9(4). 01 W-F PIC X VALUE "Y". 88 W-F-Y VALUE "Y". 88 W-F-N VALUE "N". 01 W-G COMP PIC S9(4) VALUE ZERO. 01 FILLER REDEFINES W-G. 05 FILLER PIC X. 05 W-H PIC X. 01 W-I COMP PIC S9(4). 01 W-J COMP PIC S9(4). 01 W-K COMP PIC S9(4). 01 W-L COMP PIC S9(4). 01 W-M VALUE "ABCDEF". 05 FILLER OCCURS 6 TIMES INDEXED BY I. 10 W-N PIC X. 01 W-O. 05 W-P PIC X. 05 W-Q PIC X. 01 W-R. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-K INDEXED BY J. 10 W-S PIC X. 88 W-S-Y VALUE SPACE. 01 W-T. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-K INDEXED BY K. 10 W-U PIC X. 01 W-V. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-K INDEXED BY L. 10 W-W PIC X. 01 W-X COMP PIC S9(4). 01 W-Y PIC X. 01 W-Z REDEFINES W-Y PIC 9. 01 W-AA PIC 9(4). 01 FILLER REDEFINES W-AA. 05 FILLER PIC XXX. 05 W-AB PIC 9. 01 W-AC. 05 FILLER OCCURS 1 TO 132 TIMES DEPENDING ON W-K. 10 FILLER PIC X. 01 W-AD. 05 FILLER PIC X(10) VALUE "----+----1". 05 FILLER PIC X(10) VALUE "----+----2". 05 FILLER PIC X(10) VALUE "----+----3". 05 FILLER PIC X(10) VALUE "----+----4". 05 FILLER PIC X(10) VALUE "----+----5". 05 FILLER PIC X(10) VALUE "----+----6". 05 FILLER PIC X(10) VALUE "----+----7". 05 FILLER PIC X(10) VALUE "----+----8". 05 FILLER PIC X(10) VALUE "----+----9". 05 FILLER PIC X(10) VALUE "----+----0". 05 FILLER PIC X(10) VALUE "----+----1". 05 FILLER PIC X(10) VALUE "----+----2". 05 FILLER PIC X(10) VALUE "----+----3". 05 FILLER PIC X(02) VALUE "--". LINKAGE SECTION. 01 L-A PIC X. 01 L-B REDEFINES L-A. 05 FILLER OCCURS 99999999 TIMES INDEXED BY M. 10 L-C PIC X. 01 L-D COMP PIC S9(8). 01 L-E. 05 L-F COMP PIC S9(4) VALUE +80. 88 L-F-VALID VALUE +80 +132. 05 L-G COMP PIC S9(4) VALUE ZERO. 88 L-G-Y VALUE ZERO. 05 FILLER PIC X(96) VALUE SPACE. PROCEDURE DIVISION USING L-A L-D L-E . 00-A SECTION. IF W-F-Y MOVE WHEN-COMPILED TO W-A DISPLAY "UGLY program compiled on " W-A SET W-F-N TO TRUE END-IF IF ( NOT L-F-VALID ) DISPLAY "UGLY maximum line length should be 80 or 132" DISPLAY "UGLY found>" L-F "<" CALL "FOODUMP" END-IF IF L-G-Y MOVE L-F TO W-K ELSE IF ( L-G GREATER THAN L-F ) DISPLAY "UGLY Sorry, chunks ignored, too big" DISPLAY "UGLY Chunks>" L-G "<" DISPLAY "UGLY Maxlen>" L-F "<" MOVE L-F TO W-K ELSE MOVE L-G TO W-K END-IF END-IF SET L TO W-K SET W-B TO L SET J K L TO ZERO PERFORM 00-B VARYING M FROM 1 BY 1 UNTIL M GREATER THAN L-D IF ( NOT W-S-Y ) SET W-C TO J MOVE W-C TO W-D MOVE W-E TO W-K PERFORM 00-D END-IF GOBACK . 00-B SECTION. MOVE L-C ( M ) TO W-H DIVIDE W-G BY 16 GIVING W-I REMAINDER W-J MOVE W-I TO W-X PERFORM 00-C MOVE W-Y TO W-P MOVE W-J TO W-X PERFORM 00-C MOVE W-Y TO W-Q SET J K L UP BY +1 IF ( J GREATER THAN W-B ) PERFORM 00-D MOVE SPACE TO W-R W-U SET J K L TO +1 END-IF MOVE W-P TO W-S ( J ) MOVE W-Q TO W-U ( K ) MOVE W-H TO W-W ( L ) . 00-C SECTION. IF ( W-X GREATER THAN 9 ) SUBTRACT 9 FROM W-X GIVING W-L SET I TO W-L MOVE W-N ( I ) TO W-Y ELSE MOVE W-X TO W-AA MOVE W-AB TO W-Z END-IF . 00-D SECTION. MOVE W-AD TO W-AC DISPLAY W-AC DISPLAY W-V DISPLAY W-R DISPLAY W-T .

Now, there are a couple of bugs in this version. Enjoy finding them, as you first have to work out, with no aid, what the program does.

Since you have GnuCOBOL, track down the discussion area at SourceForge.Net, the current home of GnuCOBOL.

\$\endgroup\$
1
  • \$\begingroup\$ thanks a lot. I didn't know for that "TIMES" thing. I've got to look at that. For Variable names, I've got to do better, obviously. \$\endgroup\$ Commented Dec 4, 2015 at 21:35
2
\$\begingroup\$

Some suggestions:

  • Always use descriptive variable names. Would someone new to the code know straight away what WS-RM05 or WS-RS00 refer to? In contrast, what about remainder-by-5 or div-result?
  • Use named conditions to make things clearer. Again, if you're new, would you know what WS-ML05 refers to? All that condition does is save characters and obscure the actual condition of WS-RM05 = ZERO.
  • Write identifiers in lower (or mixed) case. All caps text is generally hard to read (see this UX StackExchange question and Butterick's Practical Typography) and in COBOL, it is worth differentiating between the numerous reserved words and identifiers to tell which is which.
  • Remove the separator comments between/around division and section headers. Different divisions contain very distinctive code and you've already preceded sections by a blank line, thereby visibly separating code in different bits of the program. The separator comments are redundant. (However, if you mix sections and paragraphs in the procedure division, it may still be worth keeping them between sections for clarity.)
  • Consider removing the WS-DATA and the WS-COUNTS groups. Neither are used directly. WS-COUNTS could be replaced by a comment. Regarding WS-DATA, (I'm assuming it contains all the data to simplify initialisation), consider marking the program as INITIAL or putting items in the local-storage section instead.
  • Define constants as constants, e.g. define WS-LAST as 01 WS-LAST CONSTANT 20. or, if you're a Micro Focus user, 78 WS-LAST VALUE 20. (You have my sympathy if you're an IBM user.)
  • Use the REM intrinsic function instead of DIVIDE ... BY ... REMAINDER ... if you only need the remainder.

The following might be more personal preference than improvements:

  • Drop the leading numbers in procedure names. This seems to be a holdover from Jackson Structured Programming, which advocated a hierarchical approach to program design. The idea of a hierarchy cannot always be applied and if it is applied, the design should be intuitive enough to require no further qualification (e.g. it is logical that an output-report procedure would call the output-control-heading procedure). If not, it should be documented in comments or in a separate file, not in identifiers.
  • Remove the WS/LS/etc. prefixes as the variable name or purpose of the program should provide sufficient information to determine what section the variable is defined in.

Applying these suggestions the 10000-BEGIN and 20000-MAIN sections would become:

begin. DISPLAY "**********" . main. PERFORM compute-fizzbuzz DISPLAY ws-display-string .
\$\endgroup\$
5
  • \$\begingroup\$ Thanks. Agree with most. Leading number & prefixes I've found useful in bigger projects(especially when you need to analyze 800 programs in a row, don't laugh, I really did it, standard numbering was extremely useful), of course here they are overkill. Constants I didn't know, and neither REM. All Caps is a norm I've always seen used everywhere in COBOL, I'll try otherwise, but it will be tough. \$\endgroup\$ Commented Dec 4, 2015 at 19:41
  • 2
    \$\begingroup\$ @gazzz0x2z I would love to hear what lead to you having to read 800 programs in a row. \$\endgroup\$ Commented Dec 4, 2015 at 20:22
  • 1
    \$\begingroup\$ Read & analyze. Long story short, that shop had contract number upon 7 digits. And was already at contract number 9,500,000 and growing. So they had to rebuild everything with a 9-digits number instead. My role was to prepare the automated tests, and to check which file came from where, with what inside the datas, and what had to be compared(between old & new). Hint : Timestamps are not to be compared, for example. I also had to prepare sort parameters, and for that, understanding on surface each program making each file was important. Others made the compare tool, that ran against my params. \$\endgroup\$ Commented Dec 4, 2015 at 21:27
  • 2
    \$\begingroup\$ I strongly disagree with the 2 "personal" preference suggestions. I found section number's very useful understanding how 2 procedures relate to each other. They do not make sense in Object Orientated programming but for procedural languages (Cobol or C), they make sense. Most Cobol Shops where will be a standard (e.g. 20000 is the main program logic and it calls 210000, 220000 etc which in tern call 211000 etc). \$\endgroup\$ Commented Dec 4, 2015 at 23:04
  • 2
    \$\begingroup\$ Again I would keep the WS (or W), it distinguishes it indicates it is in working storage (this is Cobol and variables are Global). Some sites would use WC- for constants ws- general Working storage WR- reports etc. Normally there will be a site standard \$\endgroup\$ Commented Dec 4, 2015 at 23:08
2
\$\begingroup\$

You don't need IFs and you don't need modulo division.

 IDENTIFICATION DIVISION. PROGRAM-ID. fizzbuzz. DATA DIVISION. WORKING-STORAGE SECTION. 01 hooray pic x(100). 01 fred redefines hooray. 03 hoo occurs 100 pic 9(01). 01 world. 03 filler pic x(05) value spaces. 03 i pic 9(03) value 0. 03 filler pic x(24) value ' fizz buzzfizzbuzz'. 01 nurk redefines world. 03 w occurs 4 pic x(08). 01 orld pic 9(01). PROCEDURE DIVISION. move all '1' to hooray. perform varying i from 3 by 3 until i > 100 add 1 to hoo(i). perform varying i from 5 by 5 until i > 100 add 2 to hoo(i). perform varying i from 1 by 1 until i > 100 move hoo(i) to orld display w(orld). stop run.
\$\endgroup\$
0
\$\begingroup\$

OK, thanks you for all your input. I did learn quite a few things. I kept the PERFORM VARYING : I still need W-Posn to vary, and if I make a PERFORM TIMES, I'll have to add an ADD 1 TO W-POSN. I don't see the purpose. And I kept my numbering standard, as there was no consensus on the topic.

I ditched the comments & unused parts, went to Not-Totally-UpperCase names(that's an experiment for me, really looks strange, but not that bad), improved the naming(hope it's clearer now, don't know what I had in mind), removed some unused groups(but not all, as some I find useful for the clarity), used constants where needed.

And I went extreme in 2 directions :

  1. I changed the algorithm, so that I didn't have to use the remainder at all(to avoid further debate upon the function REM). It's slightly longer, but I don't need any division. It's inspired from This entry for Fizzbuzz in brainfuck.
  2. I added a few reporting elements at the beginning and at the end. For the beginning, I made 2 copybooks(data definition & code) for having a standardized reporting of standard informations as a header. For the end, I did add a few counters. Not in copybook because this is supposed to change for each program.

The copybooks for the header: HeaderData.cpy

 01 H-Header-Strings. 05 H-Line-Identity. 10 FILLER PIC X(20) VALUE "Program ". 10 H-Program-Id PIC X(20). 05 H-Line-Compile. 10 FILLER PIC X(20) VALUE "Compiled ". 10 H-Date-Compiled PIC X(08). 10 FILLER PIC X(04) VALUE " at ". 10 H-Hour-Compiled PIC X(08). 05 H-Line-Execute. 10 FILLER PIC X(20) VALUE "Executed ". 10 H-Date-Executed-mm PIC X(02). 10 FILLER PIC X(01) VALUE "/". 10 H-Date-Executed-dd PIC X(02). 10 FILLER PIC X(01) VALUE "/". 10 H-Date-Executed-yy PIC X(02). 10 FILLER PIC X(04) VALUE " at ". 10 H-Hour-Executed-hh PIC X(02). 10 FILLER PIC X(01) VALUE ".". 10 H-Hour-Executed-mn PIC X(02). 10 FILLER PIC X(01) VALUE ".". 10 H-Hour-Executed-ss PIC X(02). 05 H-Line-Char-Table. 10 FILLER PIC X(20) VALUE "Character Table ". 10 FILLER PIC X(07). 88 Char-Table-ASCII VALUE "ASCII ". 88 Char-Table-EBCDIC VALUE "EBCDIC ". 88 Char-Table-Unknown VALUE "unknown". 10 FILLER PIC X(13). 05 H-Line-OS-xxBits. 10 FILLER PIC X(20) VALUE "Turning on OS with ". 10 FILLER PIC X(07). 88 H-OS-32-Bits VALUE "32 Bits". 88 H-OS-64-Bits VALUE "64 Bits". 10 FILLER PIC X(13). 01 H-When-Compiled. 05 H-When-Compiled-Date PIC X(08). 05 H-When-Compiled-Hour PIC X(08). 01 H-When-Executed. 05 H-When-Executed-Date. 10 H-When-Executed-yy PIC 9(02). 10 H-When-Executed-mm PIC 9(02). 10 H-When-Executed-dd PIC 9(02). 05 H-When-Executed-Time. 10 H-When-Executed-hh PIC 9(02). 10 H-When-Executed-mn PIC 9(02). 10 H-When-Executed-ss PIC 9(02). 10 H-When-Executed-cc PIC 9(02). 01 H-Space-ASCII PIC X(01) VALUE X"20". 88 H-Is-ASCII VALUE SPACE. 01 H-Space-EBCDIC PIC X(01) VALUE X"40". 88 H-Is-EBCDIC VALUE SPACE. 01 H-Pointer USAGE POINTER.

HeaderDisplay.cpy

 MOVE WHEN-COMPILED TO H-When-Compiled MOVE H-When-Compiled-Date TO H-Date-Compiled MOVE H-When-Compiled-Hour TO H-Hour-Compiled ACCEPT H-When-Executed-Date FROM DATE MOVE H-When-Executed-yy TO H-Date-Executed-yy MOVE H-When-Executed-mm TO H-Date-Executed-mm MOVE H-When-Executed-dd TO H-Date-Executed-dd ACCEPT H-When-Executed-Time FROM TIME MOVE H-When-Executed-hh TO H-Hour-Executed-hh MOVE H-When-Executed-mn TO H-Hour-Executed-mn MOVE H-When-Executed-ss TO H-Hour-Executed-ss EVALUATE TRUE WHEN H-Is-ASCII SET Char-Table-ASCII TO TRUE WHEN H-Is-EBCDIC SET Char-Table-EBCDIC TO TRUE WHEN OTHER SET Char-Table-Unknown TO TRUE END-EVALUATE IF FUNCTION LENGTH(H-Pointer) EQUALS 8 SET H-OS-64-Bits TO TRUE ELSE SET H-OS-32-Bits TO TRUE END-IF DISPLAY H-Line-Identity DISPLAY H-Line-Compile DISPLAY H-Line-Execute DISPLAY H-Line-Char-Table DISPLAY H-Line-OS-xxBits .

The Code :

 IDENTIFICATION DIVISION. PROGRAM-ID. FIZZBUZZ. DATA DIVISION. WORKING-STORAGE SECTION. 01 W-First CONSTANT 0. 01 W-Last CONSTANT 20. 01 W-Algorithm-counters. 05 W-Posn PIC 9(4) VALUE ZERO. 05 W-Remainder-3 PIC 9(4) VALUE ZERO. 88 W-Remainder-Fizz-3 VALUE 3. 88 W-Remainder-Fizz-0 VALUE ZERO. 05 W-Remainder-5 PIC 9(4) VALUE ZERO. 88 W-Remainder-Buzz-5 VALUE 5. 88 W-Remainder-Buzz-0 VALUE ZERO. 01 W-Results-Counters. 05 W-Total-Printed PIC 9(4) VALUE ZERO. 05 W-Total-Numbers PIC 9(4) VALUE ZERO. 05 W-Total-Fizz PIC 9(4) VALUE ZERO. 05 W-Total-Buzz PIC 9(4) VALUE ZERO. 05 W-Total-FizzBuzz PIC 9(4) VALUE ZERO. 01 W-Display-String PIC X(10). 01 W-Display-Numeric REDEFINES W-Display-String. 05 W-FizzBuzz-Before-Int PIC X(3). 05 W-FizzBuzz-Int PIC Z(3)9. 05 W-FizzBuzz-After-Int PIC X(3). 01 W-display-Alpha REDEFINES W-Display-String. 05 W-FizzBuzz-String PIC X(10). 88 W-Is-Other VALUE "* 0000 *". 88 W-Is-Fizz VALUE "* FIZZ *". 88 W-Is-Buzz VALUE "* BUZZ *". 88 W-Is-FizzBuzz VALUE "*FIZZBUZZ*". COPY "HeaderData.cpy". PROCEDURE DIVISION. 00000-Control. PERFORM 10000-Begin PERFORM 20000-Main VARYING W-Posn FROM W-First BY 1 UNTIL W-Posn > W-Last PERFORM 30000-End . STOP RUN. 10000-Begin. MOVE "FizzBuzz" TO H-Program-Id COPY "HeaderDisplay.cpy". INITIALIZE W-Algorithm-counters W-Results-Counters DISPLAY "**********" . 20000-Main. PERFORM 40000-Create-FizzBuzzDisplay DISPLAY W-Display-String . 30000-End. DISPLAY "**********" DISPLAY "Total elements printed : " W-Total-Printed DISPLAY "Total numbers printed : " W-Total-Numbers DISPLAY "Total Fizz printed : " W-Total-Fizz DISPLAY "Total Buzz printed : " W-Total-Buzz DISPLAY "Total FizzBuzz printed : " W-Total-FizzBuzz . 40000-Create-FizzBuzzDisplay. IF W-Remainder-Fizz-3 SET W-Remainder-Fizz-0 TO TRUE END-IF IF W-Remainder-Buzz-5 SET W-Remainder-Buzz-0 TO TRUE END-IF EVALUATE TRUE WHEN (W-Remainder-Fizz-0 AND W-Remainder-Buzz-0) SET W-Is-FizzBuzz TO TRUE ADD 1 TO W-Total-FizzBuzz WHEN W-Remainder-Buzz-0 SET W-Is-Buzz TO TRUE ADD 1 TO W-Total-Buzz WHEN W-Remainder-Fizz-0 SET W-Is-Fizz TO TRUE ADD 1 TO W-Total-Fizz WHEN OTHER SET W-Is-Other TO TRUE MOVE W-Posn TO W-FizzBuzz-Int ADD 1 TO W-Total-Numbers END-EVALUATE ADD 1 TO W-Total-Printed W-Remainder-3 W-Remainder-5 . END PROGRAM FIZZBUZZ.

The resulting log :

Program FizzBuzz Compiled 12/05/15 at 22.57.08 Executed 12/05/15 at 22.57.11 Character Table ASCII Turning on OS with 32 Bits ********** *FIZZBUZZ* * 1 * * 2 * * FIZZ * * 4 * * BUZZ * * FIZZ * * 7 * * 8 * * FIZZ * * BUZZ * * 11 * * FIZZ * * 13 * * 14 * *FIZZBUZZ* * 16 * * 17 * * FIZZ * * 19 * * BUZZ * ********** Total elements printed : 0021 Total numbers printed : 0011 Total Fizz printed : 0005 Total Buzz printed : 0003 Total FizzBuzz printed : 0002

Of course, it's insane for a FizzBuzz : 88 lines in the main code, with 66 + 33 lines in the copybooks, for a grand total of 187 LOC. But I've got a good starting point for new adventures, I hope.

Once again, thanks for your great entries, both. I've got to choose a "winner" answer now, and it's not easy. I'll go for Bill Woodger for its great examples - even if I'm not going to copy its style anyways. I did learn quite a few things reading his code. There is always room to grow.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Of course, with TIMES you need to take account of things like W-POSN yourself. That's what I meant my "more code". You're using VARYING just to do that, whereas nothing varies about the termination condition, nothing varies about the data. Starting to look very good. \$\endgroup\$ Commented Dec 6, 2015 at 9:55
0
\$\begingroup\$

I think this is better:

IDENTIFICATION DIVISION. PROGRAM-ID. FIZZ-BUZZ. DATA DIVISION. WORKING-STORAGE SECTION. 01 WS-STR PIC X(30). 01 WS-CNT PIC 9(03). 01 WS-DEVIDENT PIC 9(03). 01 WS-REMAINDER PIC 9(03). PROCEDURE DIVISION. FIRST-PARA. PERFORM VARYING WS-CNT FROM 1 BY 1 UNTIL WS-CNT >= 20 INITIALIZE WS-STR DIVIDE WS-CNT BY 3 GIVING WS-DEVIDENT REMAINDER WS-REMAINDER IF WS-REMAINDER = 0 STRING WS-STR DELIMITED BY SPACE "FIZZ" DELIMITED BY SIZE INTO WS-STR END-STRING END-IF DIVIDE WS-CNT BY 5 GIVING WS-DEVIDENT REMAINDER WS-REMAINDER IF WS-REMAINDER = 0 STRING WS-STR DELIMITED BY SPACE "BUZZ" DELIMITED BY SIZE INTO WS-STR END-STRING END-IF IF WS-STR = SPACES MOVE WS-CNT TO WS-STR END-IF DISPLAY WS-STR END-PERFORM STOP RUN.
\$\endgroup\$
2
  • 3
    \$\begingroup\$ Could you please explain why your approach is better ? \$\endgroup\$ Commented Jan 14, 2018 at 12:40
  • \$\begingroup\$ Well, there is not "better" or "worse", there are different styles for different reasons. Your style is obviously "straight to goal". Mine was to have a comprehensive ecosystem, with not only the answer to the problem, but also all the bells & whistles to be part of a greater system. Ah, and you mess a "." before "STOP RUN." \$\endgroup\$ Commented Jan 15, 2018 at 12:44

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.