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.
-
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\$Hans Kilian– Hans Kilian2024年01月23日 09:25:11 +00:00Commented 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\$radarbob– radarbob2024年01月24日 21:01:16 +00:00Commented 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\$drake14w– drake14w2024年01月24日 23:05:47 +00:00Commented Jan 24, 2024 at 23:05
1 Answer 1
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.
-
4\$\begingroup\$ I hope you’re not a teacher... \$\endgroup\$not2savvy– not2savvy2024年01月23日 08:18:36 +00:00Commented 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\$mishan– mishan2024年01月23日 15:12:09 +00:00Commented Jan 23, 2024 at 15:12
-
4\$\begingroup\$ @not2savvy I hope you're not the ones evaluating the teachers. \$\endgroup\$user279713– user2797132024年01月24日 04:17:09 +00:00Commented 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\$Martijn– Martijn2024年01月24日 16:19:45 +00:00Commented 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\$not2savvy– not2savvy2024年01月24日 20:15:58 +00:00Commented Jan 24, 2024 at 20:15