10
\$\begingroup\$

For a school project I've created a database program in COBOL and now that I'm finished, I have to write a report on the project.

As one aspect of the report, we have to discuss whether the product (the program) achieved all of the "success criteria" we created earlier in the process. One of my success criteria was code style and readability. More specifically: "The source code of the final product must be easily readable, clear, and logical."

I wanted to get an outside opinion on this so I've come to this website.

       IDENTIFICATION DIVISION.
       PROGRAM-ID. COBDB.

       ENVIRONMENT DIVISION.
       INPUT-OUTPUT SECTION.
       FILE-CONTROL.
           SELECT OPTIONAL DB-FILE
           ASSIGN USING DB-FILE-NAME
           ORGANIZATION IS SEQUENTIAL
           ACCESS IS SEQUENTIAL.

       DATA DIVISION.
       FILE SECTION.
       FD DB-FILE.
           01 CHAR PIC X(1).

       WORKING-STORAGE SECTION.
       01 PARSING-DATA.
           05 DB-FILE-NAME-INDEX            PIC 9(3) USAGE IS
                                            COMPUTATIONAL.
           05 PARSING-DONE                  PIC 9(1).
           05 NUM-RECORDS-PARSING-START     PIC 9(1).
           05 NUM-RECORDS-PARSING-STARTED   PIC 9(1).
           05 NUM-RECORDS-PARSING-INDEX     PIC 9(2).
           05 DB-FILE-NAME                  PIC X(500).
       01 DATA-REGISTERS.
           05 DATA-REGISTER                 PIC X(4096).
           05 LENGTH-REGISTER               PIC 9(4) USAGE IS
                                            DISPLAY.
       01 HEADER-DATA.
           05 ROW-NUM                       USAGE IS COMP-2.
           05 FILE-INITIALIZED              PIC 9(1).
           05 FILE-EMPTY                    PIC 9(1).
           05 HEADER-PARSE-STARTED          PIC 9(1).
           05 NUM-RECORDS                   PIC 9(18).
           05 COL-VALS                      OCCURS 1 TO 4096 TIMES
                                            DEPENDING ON ROW-NUM INDEXED
                                            BY COL-VALS-INDEX.
                   10 COL-VAL               PIC 9(1) USAGE IS COMP-3. 
       01 INITIALIZATION-DATA.
           05 INPUT-COL-VALS-INDEX          PIC 9(4).
           05 INITIALIZATION-STARTED        PIC 9(1).
           05 INITIALIZATION-FAILED         PIC 9(1).
       01 MISC-DATA.
           05 EOF                           PIC 9(1).
           05 INPUT-DATA                    PIC X(4096).
           05 DATA-MODE                     PIC 9(1).
           05 DIVIDER                       PIC X(80).
           05 COMMAND                       PIC X(10).
       01 WRITE-PARSING-DATA.
           05 WRITE-PARSING-INDEX           PIC 9(4).
           05 WRITE-PARSING-STARTED         PIC 9(1).
           05 WRITE-PARSING-FAILED          PIC 9(1).
           05 WRITE-PARSING-LEN             PIC 9(4).
           05 WRITE-PARSING-LEN-INDEX       PIC 9(1).
           05 WRITE-PARSING-VALS-CURRENTLY  PIC 9(1).
           05 WRITE-PARSING-LEN-CURRENTLY   PIC 9(1).
           05 WRITE-PARSING-CHAR            PIC X(1).
           05 WRITE-PARSING-CHAR-AS-INT
           REDEFINES WRITE-PARSING-CHAR     PIC 9(1).
           05 FINAL-CHARACTER-INDEX         PIC 9(4).
       01 READ-DEL-PARSING-DATA.
           05 SEARCH-KEY                    PIC 9(18).
           05 COLS-INDEX                    PIC 9(4).
           05 READING-LEN-INDEX             PIC 9(1).
           05 SEARCH-FAILED                 PIC 9(1).
           05 SEARCH-DONE                   PIC 9(1).
           05 READING-LEN                   PIC 9(1).
           05 READDEL-VAL                   PIC 9(1).
           05 REGISTER-INDEX                PIC 9(4).
           05 PARSING-DELED-REC             PIC 9(1).

       PROCEDURE DIVISION.
       MAIN.
           PERFORM INIT-DATA.
           PERFORM GET-INPUT.
           PERFORM OPEN-FILE.
           INITIALIZE COMMAND.
           DISPLAY "Please enter command"
           ACCEPT COMMAND.
           IF COMMAND = "INITIALIZE"
              DISPLAY "Please enter input"
              PERFORM INITIALIZE-FILE
           ELSE IF COMMAND = "WRITE     "
              DISPLAY "Please enter input"
              PERFORM WRITE-REC
           ELSE IF COMMAND = "READ      "
              DISPLAY "Please enter input"
              PERFORM READ-REC
           ELSE IF COMMAND = "DELETE    "
              DISPLAY "Please enter input"
              PERFORM DELETE-REC
           ELSE
              DISPLAY "Error: Invalid command"
           END-IF.
           STOP RUN.

       INIT-DATA.
           INITIALIZE PARSING-DATA.
           INITIALIZE DATA-REGISTERS.
           INITIALIZE HEADER-DATA.
           INITIALIZE MISC-DATA.
           INITIALIZE INITIALIZATION-DATA.
           INITIALIZE WRITE-PARSING-DATA.
           INITIALIZE READ-DEL-PARSING-DATA.

       GET-INPUT.
           ACCEPT DB-FILE-NAME FROM COMMAND-LINE.
           MOVE 501 TO DB-FILE-NAME-INDEX.
           PERFORM UNTIL PARSING-DONE = 1
               SUBTRACT 1 FROM DB-FILE-NAME-INDEX
               IF NOT DB-FILE-NAME (DB-FILE-NAME-INDEX : 1) = ' '
                   MOVE 1 TO PARSING-DONE
               END-IF
           END-PERFORM.

       OPEN-FILE.
           SET COL-VALS-INDEX TO 1.
           MOVE 1 TO FILE-EMPTY
           MOVE 1 TO NUM-RECORDS-PARSING-INDEX
           OPEN INPUT DB-FILE.
               PERFORM UNTIL FILE-INITIALIZED = '1' OR EOF = '1' OR
               CHAR = '$'
                   READ DB-FILE
                       AT END
                           MOVE '1' TO EOF
                           IF (FILE-EMPTY = 0) AND 
                           (FILE-INITIALIZED = 0) AND
                           (NUM-RECORDS-PARSING-STARTED = 0)
                                DISPLAY "File is corrupted. Please ensur
      -                         "e that you are inputting a proper cobDB
      -                         " file or an empty/non existant file. Ex
      -                         "iting...1"
                                CLOSE DB-FILE
                                EXIT PROGRAM
                           END-IF
                       NOT AT END
                           IF FILE-EMPTY = 1
                               MOVE 0 TO FILE-EMPTY
                           END-IF
                           IF HEADER-PARSE-STARTED = 0
                               IF CHAR = '@'
                                   MOVE 1 TO HEADER-PARSE-STARTED 
                               ELSE   
                                   DISPLAY "File is corrupted. Please en
      -                            "sure that you are inputting a proper
      -                            " cobDB file or an empty/non existant
      -                            " file. Exiting...2"
                                   CLOSE DB-FILE
                                   EXIT PROGRAM
                               END-IF
                           ELSE IF NUM-RECORDS-PARSING-START = 1
                               IF CHAR IS NUMERIC
                                   MOVE CHAR TO NUM-RECORDS
                                   (NUM-RECORDS-PARSING-INDEX : 1)
                                   ADD 1 TO NUM-RECORDS-PARSING-INDEX
                                   IF NUM-RECORDS-PARSING-STARTED = 0
                                       MOVE 1 TO
                                       NUM-RECORDS-PARSING-STARTED
                                   END-IF
                               ELSE IF CHAR = '$'
                                   CONTINUE
                               ELSE
                                   DISPLAY "0"
                                   CLOSE DB-FILE
                                   EXIT PROGRAM
                               END-IF
                           ELSE
                               IF CHAR IS NUMERIC
                                   MOVE CHAR TO COL-VALS 
                                   (COL-VALS-INDEX)
                                   SET COL-VALS-INDEX UP BY 1
                               ELSE IF CHAR = '#'
                                   MOVE 1 TO NUM-RECORDS-PARSING-START
                               ELSE
                                   DISPLAY "File is corrupted. Please en
      -                            "sure that you are inputting a proper
      -                            " cobDB file or an empty/non existant
      -                            " file. Exiting...3"
                                   CLOSE DB-FILE
                                   EXIT PROGRAM
                               END-IF
                           END-IF
                   END-READ
               END-PERFORM.
           CLOSE DB-FILE.

       INITIALIZE-FILE.
           ACCEPT INPUT-DATA.
           MOVE 0 TO INITIALIZATION-STARTED.
           OPEN OUTPUT DB-FILE.
               MOVE '@' TO CHAR.
               WRITE CHAR END-WRITE.
               PERFORM VARYING INPUT-COL-VALS-INDEX FROM 1 BY 1 UNTIL
               INPUT-COL-VALS-INDEX IS GREATER THAN 4096
                   EVALUATE INPUT-DATA (INPUT-COL-VALS-INDEX : 1)
                       WHEN 'I'
                           MOVE '1' TO CHAR
                           WRITE CHAR END-WRITE
                           IF INITIALIZATION-STARTED = 0
                               MOVE 1 TO INITIALIZATION-STARTED
                           END-IF
                       WHEN 'F'
                           MOVE '2' TO CHAR
                           WRITE CHAR END-WRITE 
                           IF INITIALIZATION-STARTED = 0
                               MOVE 1 TO INITIALIZATION-STARTED
                           END-IF
                       WHEN 'S'
                           MOVE '3' TO CHAR
                           WRITE CHAR END-WRITE 
                           IF INITIALIZATION-STARTED = 0
                               MOVE 1 TO INITIALIZATION-STARTED
                           END-IF
                       WHEN ' ' 
                           IF INITIALIZATION-STARTED = 0
                               DISPLAY "Error: Initialization failed. In
      -                        "itialization string was not properly for
      -                        "matted."
                               MOVE 1 TO INITIALIZATION-FAILED
                           END-IF
                           NEXT SENTENCE
                       WHEN OTHER
                           DISPLAY "Error: Initialization failed. Initia
      -                    "lization string was not properly formatted."
                           MOVE 1 TO INITIALIZATION-FAILED
                           CLOSE DB-FILE
                           NEXT SENTENCE
                   END-EVALUATE
               END-PERFORM.
               IF INITIALIZATION-FAILED = 0
                   MOVE '#' TO CHAR
                   WRITE CHAR END-WRITE
                   MOVE 0 TO NUM-RECORDS
                   PERFORM 18 TIMES
                       MOVE '0' TO CHAR
                       WRITE CHAR END-WRITE
                   END-PERFORM
                   MOVE '$' TO CHAR
                   WRITE CHAR END-WRITE
               END-IF.
           CLOSE DB-FILE.

       WRITE-REC.
           ACCEPT INPUT-DATA.
           IF INITIALIZATION-FAILED = 1
               DISPLAY "Error" 
           END-IF.
           MOVE 0 TO WRITE-PARSING-STARTED.
           MOVE 0 to WRITE-PARSING-FAILED.
           MOVE 0 TO WRITE-PARSING-LEN-INDEX.
           MOVE 0 TO WRITE-PARSING-VALS-CURRENTLY.
           MOVE 0 TO WRITE-PARSING-LEN-CURRENTLY.
           PERFORM VARYING WRITE-PARSING-INDEX FROM 1 BY 1 UNTIL
                   WRITE-PARSING-INDEX IS GREATER THAN 4096
               MOVE INPUT-DATA (WRITE-PARSING-INDEX : 1) TO
               WRITE-PARSING-CHAR
               IF WRITE-PARSING-STARTED = 0
                   IF WRITE-PARSING-CHAR IS NUMERIC
                      ADD 1 TO WRITE-PARSING-LEN-INDEX
                      MOVE WRITE-PARSING-CHAR-AS-INT TO 
                      WRITE-PARSING-LEN (WRITE-PARSING-LEN-INDEX : 1)
                      MOVE 0 TO WRITE-PARSING-VALS-CURRENTLY
                      MOVE 1 TO WRITE-PARSING-LEN-CURRENTLY
                      MOVE 1 TO WRITE-PARSING-STARTED
                   ELSE
                      MOVE 1 TO WRITE-PARSING-FAILED
                      DISPLAY "2"
                      NEXT SENTENCE
                   END-IF
               ELSE
                   IF WRITE-PARSING-LEN-CURRENTLY = 1
                       IF WRITE-PARSING-CHAR IS NUMERIC
                           ADD 1 TO WRITE-PARSING-LEN-INDEX
                           MOVE WRITE-PARSING-CHAR-AS-INT TO
                           WRITE-PARSING-LEN (WRITE-PARSING-LEN-INDEX :
                           1)
                           IF WRITE-PARSING-LEN-INDEX = 4
                               MOVE 0 TO WRITE-PARSING-LEN-CURRENTLY
                               MOVE 1 TO WRITE-PARSING-VALS-CURRENTLY
                               MOVE 0 TO WRITE-PARSING-LEN-INDEX
                           END-IF
                       ELSE
                           MOVE 1 TO WRITE-PARSING-FAILED
                           DISPLAY "3"
                           NEXT SENTENCE
                       END-IF
                   ELSE IF WRITE-PARSING-VALS-CURRENTLY = 1
                       SUBTRACT 1 FROM WRITE-PARSING-LEN
                       IF WRITE-PARSING-LEN = 0
                           MOVE 0 TO WRITE-PARSING-VALS-CURRENTLY
                       END-IF
                   ELSE
                       IF WRITE-PARSING-CHAR IS NUMERIC
                           ADD 1 TO WRITE-PARSING-LEN-INDEX
                           MOVE WRITE-PARSING-CHAR-AS-INT TO
                           WRITE-PARSING-LEN (WRITE-PARSING-LEN-INDEX :
                           1)
                           MOVE 1 TO WRITE-PARSING-LEN-CURRENTLY
                       ELSE IF WRITE-PARSING-CHAR = ' '
                           MOVE WRITE-PARSING-INDEX TO
                           FINAL-CHARACTER-INDEX
                           NEXT SENTENCE
                       ELSE
                           MOVE 1 TO WRITE-PARSING-FAILED
                           DISPLAY "4"
                           NEXT SENTENCE
                       END-IF
                   END-IF    
               END-IF
           END-PERFORM.
           IF WRITE-PARSING-FAILED = 0
               OPEN EXTEND DB-FILE
                   PERFORM VARYING WRITE-PARSING-INDEX FROM 1 BY 1 UNTIL
                   WRITE-PARSING-INDEX = FINAL-CHARACTER-INDEX
                       MOVE INPUT-DATA (WRITE-PARSING-INDEX : 1) TO CHAR
                       WRITE CHAR END-WRITE
                   END-PERFORM
               CLOSE DB-FILE
               ADD 1 TO NUM-RECORDS
               OPEN I-O DB-FILE
                   PERFORM UNTIL CHAR = '#'
                       READ DB-FILE
                   END-PERFORM
                   PERFORM VARYING WRITE-PARSING-INDEX FROM 1 BY 1 
                   UNTIL WRITE-PARSING-INDEX IS GREATER THAN 18
                       READ DB-FILE END-READ
                       MOVE NUM-RECORDS (WRITE-PARSING-INDEX : 1) TO
                       CHAR
                       REWRITE CHAR
                   END-PERFORM
               CLOSE DB-FILE
           END-IF.

       READ-REC.
           MOVE ALL '_' TO DIVIDER (1 : 80).
           MOVE 0 TO SEARCH-FAILED.
           MOVE 0 TO SEARCH-DONE.
           MOVE 1 TO READING-LEN.
           MOVE 1 TO READING-LEN-INDEX.
           MOVE 0 TO READDEL-VAL.
           MOVE 1 TO REGISTER-INDEX.
           MOVE 0 TO PARSING-DELED-REC.
           MOVE COL-VALS-INDEX TO COLS-INDEX.
           SUBTRACT 1 FROM COLS-INDEX.
           MOVE ' ' TO CHAR.
           ACCEPT SEARCH-KEY.
           IF SEARCH-KEY IS GREATER THAN (NUM-RECORDS - 1)
               DISPLAY "6"
               EXIT
           END-IF.
           OPEN INPUT DB-FILE.
               PERFORM UNTIL CHAR = '$'
                  READ DB-FILE 
               END-PERFORM
               PERFORM UNTIL (SEARCH-DONE = 1) OR (SEARCH-FAILED = 1) 
                   READ DB-FILE    
                   IF PARSING-DELED-REC = 1
                       IF CHAR = '^'
                           MOVE 0 TO PARSING-DELED-REC
                           MOVE 1 TO READING-LEN
                           MOVE 0 TO READDEL-VAL
                           MOVE 1 TO READING-LEN-INDEX
                       END-IF
                   ELSE IF READING-LEN = 1
                       IF CHAR IS NUMERIC
                           IF CHAR = '9'
                               IF READING-LEN-INDEX = 1
                                   MOVE 1 TO PARSING-DELED-REC
                               END-IF
                           END-IF
                           MOVE CHAR TO LENGTH-REGISTER
                           (READING-LEN-INDEX : 1)
                           ADD 1 TO READING-LEN-INDEX
                           IF READING-LEN-INDEX = 5
                               MOVE 1 TO READING-LEN-INDEX
                               MOVE 0 TO READING-LEN
                               MOVE 1 TO READDEL-VAL
                               IF DATA-MODE = 1
                                   DISPLAY LENGTH-REGISTER
                               END-IF
                           END-IF 
                       ELSE
                           DISPLAY "7"
                           MOVE 1 TO SEARCH-FAILED
                       END-IF
                   ELSE IF READDEL-VAL = 1
                       IF SEARCH-KEY IS GREATER THAN 0
                           SUBTRACT 1 FROM LENGTH-REGISTER
                           IF LENGTH-REGISTER = 0
                               MOVE 1 TO READING-LEN
                               MOVE 0 TO READDEL-VAL
                               SUBTRACT 1 FROM COLS-INDEX
                               IF COLS-INDEX = 0
                                   MOVE COL-VALS-INDEX TO COLS-INDEX
                                   SUBTRACT 1 FROM COLS-INDEX
                                   SUBTRACT 1 FROM SEARCH-KEY
                               END-IF
                           END-IF
                       ELSE
                           MOVE CHAR TO DATA-REGISTER (REGISTER-INDEX :
                           1)
                           SUBTRACT 1 FROM LENGTH-REGISTER
                           ADD 1 TO REGISTER-INDEX
                           IF LENGTH-REGISTER = 0
                               MOVE 1 TO READING-LEN
                               MOVE 0 TO READDEL-VAL
                               SUBTRACT 1 FROM COLS-INDEX
                               DISPLAY DATA-REGISTER (1 : 
                               REGISTER-INDEX)
                               MOVE 1 TO REGISTER-INDEX
                               MOVE ALL ' ' TO DATA-REGISTER (1 : 4096)
                               IF DATA-MODE = 0
                                   DISPLAY DIVIDER
                               END-IF
                               IF COLS-INDEX = 0
                                   MOVE 1 TO SEARCH-DONE
                               END-IF
                           END-IF
                       END-IF
               END-PERFORM.
           CLOSE DB-FILE.
       DELETE-REC.
           MOVE 0 TO SEARCH-FAILED.
           MOVE 0 TO SEARCH-DONE.
           MOVE 1 TO READING-LEN.
           MOVE 1 TO READING-LEN-INDEX.
           MOVE 0 TO READDEL-VAL.
           MOVE COL-VALS-INDEX TO COLS-INDEX.
           SUBTRACT 1 FROM COLS-INDEX.
           MOVE ' ' TO CHAR.
           ACCEPT SEARCH-KEY.
           IF SEARCH-KEY IS GREATER THAN (NUM-RECORDS - 1)
               DISPLAY "6"
               EXIT
           END-IF.
           OPEN I-O DB-FILE.
               PERFORM UNTIL CHAR = '$'
                  READ DB-FILE 
               END-PERFORM.
               PERFORM UNTIL (SEARCH-DONE = 1) OR (SEARCH-FAILED = 1)
                   READ DB-FILE
                   IF READING-LEN = 1
                       IF CHAR IS NUMERIC
                           MOVE CHAR TO LENGTH-REGISTER
                           (READING-LEN-INDEX : 1)
                           ADD 1 TO READING-LEN-INDEX
                           IF READING-LEN-INDEX = 5
                               MOVE 1 TO READING-LEN-INDEX
                               MOVE 0 TO READING-LEN
                               MOVE 1 TO READDEL-VAL
                               IF DATA-MODE = 1
                                   DISPLAY LENGTH-REGISTER
                               END-IF
                           END-IF
                           IF SEARCH-KEY = 0
                               MOVE '9' TO CHAR
                               REWRITE CHAR END-REWRITE
                           END-IF
                       ELSE
                           DISPLAY "7"
                           MOVE 1 TO SEARCH-FAILED
                       END-IF
                   ELSE IF READDEL-VAL = 1
                       IF SEARCH-KEY IS GREATER THAN 0
                           SUBTRACT 1 FROM LENGTH-REGISTER
                           IF LENGTH-REGISTER = 0
                               MOVE 1 TO READING-LEN
                               MOVE 0 TO READDEL-VAL
                               SUBTRACT 1 FROM COLS-INDEX
                               IF COLS-INDEX = 0
                                   MOVE COL-VALS-INDEX TO COLS-INDEX
                                   SUBTRACT 1 FROM COLS-INDEX
                                   SUBTRACT 1 FROM SEARCH-KEY
                               END-IF
                           END-IF
                       ELSE
                           SUBTRACT 1 FROM LENGTH-REGISTER
                           IF LENGTH-REGISTER = 0
                               MOVE '^' TO CHAR
                               REWRITE CHAR END-REWRITE
                               MOVE 1 TO READING-LEN
                               MOVE 0 TO READDEL-VAL
                               SUBTRACT 1 FROM COLS-INDEX
                               IF COLS-INDEX = 0
                                   MOVE 1 TO SEARCH-DONE
                               END-IF
                           ELSE
                               MOVE ' ' TO CHAR
                               REWRITE CHAR END-REWRITE
                           END-IF
                       END-IF
                   END-IF
               END-PERFORM.
           CLOSE DB-FILE.
\$\endgroup\$
3
  • 1
    \$\begingroup\$ One thing that I noticed is that, in your MAIN routine, you call GET-INPUT. And then, after that, you spend some effort getting some input (command etc.). That confused me. Either rename GET-INPUT to a more specific name or move the getting of the command into GET-INPUT. \$\endgroup\$ Jan 23 at 9:25
  • \$\begingroup\$ Is the program launched each time you want to enter a command? Is this because of the computer it's running on? We had a Burroughs time sharing system that worked that way. \$\endgroup\$
    – radarbob
    Jan 24 at 21:01
  • \$\begingroup\$ @radarob I did this because I was pressed for time when finishing this project and this was the quickest way to implement the user input. \$\endgroup\$
    – drake14w
    Jan 24 at 23:05

1 Answer 1

20
\$\begingroup\$

ZOMG, that is frightening along so many dimensions.

Disclaimer: Within the last decade I wrote lots of python code to parse production COBOL database record formats. And as an undergraduate I had roommates who studied COBOL in a non-ironic way.

format

Ok, style points for leaving room for six-digit card sequence numbers, plus a blank.

But, didn't you want to start out by telling us what language dialect you were coding to? Like, IDK, COBOL-2014 or something? I hear you don't even have to use ALL CAPS EBCDIC any more.

This feels more like I'm reading a green-bar printout in the lobby of the Computer History Museum than reading a recently authored source file on github.

review context

I'm not seeing any comments, and maybe that's not an entirely bad thing. But the OP submission included no ReadMe.md file, and no narrative remarks about what problem we're trying to solve, what approach we took, and what the pre-defined success criteria are.

In a word, there's no contract. Such promises mattered to Rear Adm. Grace Hopper, and they matter to me. If someone has to maintain what you wrote, it will matter to them.

arbitrarily out-of-sequence

       INIT-DATA.
           ...
           INITIALIZE HEADER-DATA.
           INITIALIZE MISC-DATA.
           INITIALIZE INITIALIZATION-DATA.

I don't like this.

It corresponds to:

       01 HEADER-DATA.
           ... 
       01 INITIALIZATION-DATA.
           ...
       01 MISC-DATA.

We have nightmarish DRY issues all over the place as a design relic from 1958, and no automated support from awk, m4, or similar preprocessors, so you're asking humans to do the manual error checking. To randomly reorganize the order is not a kindness to the Gentle Reader.

magic number

       GET-INPUT.
           ...
           MOVE 501 TO DB-FILE-NAME-INDEX.

WTF, where did 501 come from? We don't like naming our magic constants? My difficulty is, I simply don't know what it means, and grep'ing for 501 fails. OIC, the related DB-FILE-NAME can hold 500 characters. What, there's no LENGTH-OF-STORAGE-ELEMENT(DB-FILE-NAME) defined in this dialect of the language? Ok, fine, it is what it is.

The GET-INPUT identifier is kind of true, but it doesn't tell the whole story, we might have gone with a longer identifier. Scanning for last non-blank filename character seems to be most of what it's responsible for. Apparently you had the opportunity to put an * asterisk in column seven, followed by descriptive comments, and you elected not to do so.

trim

I was going to suggest writing a TRIM-BLANKS utility function.

But it turns out that folks working with modern IBM COBOL products will use FUNCTION LENGTH(DB-FILE-NAME-INDEX) to obtain the integer 500, and will use FUNCTION TRIM(db-file-name-index TRAILING) to strip whitespace from the end, for example during a MOVE. (I was just adding emphasis -- compiler likely wants to see upcased identifier.)

We've had 65 years to perfect this operation. I recommend you avoid re-inventing the wheel.

open-file

               PERFORM UNTIL FILE-INITIALIZED = '1' OR ...
                           IF (FILE-EMPTY = 0) AND ...

Sorry, I'm getting distracted by the gratuitous datatype changes. We declared both of those as PIC 9(1). Yet sometimes we talk about them as character variables, and sometimes as numeric variables. Yes, I understand the language lets you get away with it. But if you're trying to communicate a concept to a human reader, it's better to stick to just a single convention. Prefer to think of booleans as numeric, rather than as character data.

Also, I bet I don't even want to know about order-of-operations, and how tightly AND and OR bind. Your use of ( ) parens for conjuncts and disjuncts is inconsistent, which makes me nervous about what the underlying language rules might be.

EDIT:

It turns out that OR binds least tightly, as you might expect, so that's OK.
For example a AND b OR c AND d is (a AND b) OR (c AND d).

diagnostic message

                                DISPLAY "File is corrupted. Please ensur
      -                         "e that you are inputting a proper cobDB
      -                         " file or an empty/non existant file. Ex
      -                         "iting...1"

The grammar here is not great.

Always remember: the goal of a diagnostic is to tell the user what to do to Win, not to advise him that he is Losing.

Here, the verb "ensure" is too passive. Be direct, tell the user what to do! Please input a proper.... Or better, please specify, though perhaps "input" is part of the relevant computing culture here, and will be better understood.

Also, DRY, this diagnostic appears multiple times. Create a symbolic constant, which you DISPLAY as needed.

grammar

It seems clear that OPEN-FILE, despite the misleading name, recognizes some context free grammar. Which one, however, is obscure.

Recall that you can put an * asterisk in column 7, and then offer any helpful comments that might spring to mind.

Perhaps an EBNF grammar, or a regex, would be a concise way of explaining what this procedure promises to do.

output specification

                                   DISPLAY "0"

I have no idea what to make of that.

Perhaps there was some Requirements Document? Which you did not include in the OP submission?

If there's a requirement to report success in this fashion, then the source or the review context should make that clear.

helpful prompts

              DISPLAY "Please enter input"
           ...
              DISPLAY "Please enter input"
           ...
              DISPLAY "Please enter input"
           ...
              DISPLAY "Please enter input"

From a UX perspective, these are just terrible.

You should at least remind me that I should enter {init, write, read, del} input.

But, I have to believe, surely there are differences in those inputs, which you might helpfully remind me of?


automated tests

I have no idea what the current state-of-the-art is for COBOL unit tests. But certainly no such tests were offered in the OP submission.

A suite of passing tests improves our level of confidence that code works correctly for the regular and for the corner cases. It gives us the confidence to maintain and refactor existing code, so we can see that after an edit we still get a Green bar. It is an educational resource that assists in quicky onboarding new colleagues to the project.


I am willing to believe this code achieves some of its design goals. Those goals did not appear in the OP submission so it is hard to be certain.

I might be willing to delegate maintenance tasks on this code to a colleague, who is well versed in COBOL. I would definitely not be willing to maintain this code in its current form, absent tests and project documentation.

\$\endgroup\$
6
  • 4
    \$\begingroup\$ I hope you’re not a teacher… \$\endgroup\$
    – not2savvy
    Jan 23 at 8:18
  • 9
    \$\begingroup\$ Eh..., he's grumpily grimly growling going through the submission, but in the end does give him a code review and points out where to look for improvements. \$\endgroup\$
    – mishan
    Jan 23 at 15:12
  • 4
    \$\begingroup\$ @not2savvy I hope you're not the ones evaluating the teachers. \$\endgroup\$
    – Voile
    Jan 24 at 4:17
  • 1
    \$\begingroup\$ @not2savvy while grumpy, I'd like to point out that all the comments are at the code, not to the person who wrote it. It maybe done a little more tactful, but IMO this is fine :) \$\endgroup\$
    – Martijn
    Jan 24 at 16:19
  • \$\begingroup\$ @Martijn I’m not at all saying the comments are wrong, it’s only that I hope that how it’s being said is not demotivating the questioner, which would be a pity. \$\endgroup\$
    – not2savvy
    Jan 24 at 20:15

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.