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.
Sᴀᴍ Onᴇᴌᴀ
29.6k16 gold badges45 silver badges203 bronze badges
asked Jan 23, 2024 at 0:48
\$\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\$ Commented Jan 23, 2024 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\$ Commented Jan 24, 2024 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\$ Commented Jan 24, 2024 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.

answered Jan 23, 2024 at 3:03
\$\endgroup\$
6
  • 4
    \$\begingroup\$ I hope you’re not a teacher... \$\endgroup\$ Commented Jan 23, 2024 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\$ Commented Jan 23, 2024 at 15:12
  • 4
    \$\begingroup\$ @not2savvy I hope you're not the ones evaluating the teachers. \$\endgroup\$ Commented Jan 24, 2024 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\$ Commented Jan 24, 2024 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\$ Commented Jan 24, 2024 at 20:15

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.