9
\$\begingroup\$

Whenever I learn a new (or an old :)) programming language, I implement Hangman in it. This time it's with COBOL.

identification division.
program-id. hangman.

data division.
working-storage section.
01 state.
    05 word pic A(100).
    05 word-length pic 9(3).
    05 guess pic A.
    05 guesses occurs 256 times pic 9.
    05 done pic 9.
01 i pic 9(3).

procedure division.
* TODO: pick random word from word list
    move "hello" to word
    move 5 to word-length

* TODO: show this in debug mode only
    display "word: " word

    perform until done = 1
        accept guess
        move 1 to guesses(function ord(guess))
        move 1 to done
        perform varying i from 1 by 1 until i > word-length
            if guesses(function ord(word(i:1))) = 1 then
                display word(i:1) with no advancing
            else
                move 0 to done
                display "_" with no advancing
            end-if
        end-perform
        display " "
    end-perform.

I'm of course terrible at COBOL, and I wonder whether the data types I used for the variables are the correct ones and whether there are better ones to use instead. I'm also worried about the hardcoded size for word. I'm using the latest GnuCOBOL.

\$\endgroup\$
5
\$\begingroup\$

In the Hangman I know, there is a limit to the number of incorrect guesses :-), and a little diagram to show how close to failing you are...

I've looked at your program, and can't break it (except that the program's method of stopping is to "fall off the end", which I've not tried but I assume must be OK in GnuCOBOL 2), so that's good, but it is only a short program. There's not much particularly "COBOL" about it, which is a criticism of the task chosen and its implementation, rather than the code.

There are no "rules" about how to write your own COBOL programs, but "in action" you would have some local standards about how things are done. These standards are rarely perfect, but at least other members of the team can pick up someone's program and not have to understand the individual style.

Data-names/identifiers and labels: Make these descriptive; group definitions together which are related

Data-types: PIC A(...) does not get used except by beginners; don't use PIC 9(...) just because something is "a number", unless it is used where a numeric definition is required, use PIC X(...)

Data sizes, descriptions and validation: use what is needed, no more, and no less. That PIC A(100) will confuse a reader. What is that for? Is it really only a WORD? Or slackly used for something else? Limit it, and describe it to your user ("You have to guess a word which is a maximum of N letters long"). You have 256 entries for guesses when only 52 are needed (upper- and lower-case letters). Why process user-input which is not valid?

Formatting/indenting: use it well and consistently. I arrange things more like @Kwebble mentioned in their comment. Use your editor effectively to aid you to write code for the human reader.

Full-stops/periods in the PROCEDURE DIVISION: as was mentioned, keep these to a minimum, and don't attach any to actual code. This will ease copy/pasting of code, as you will never have to decide whether the new location requires the full-stop/period or not.

88-level/condition-names: use them. Don't use literals in the PROCEDURE DIVISION for tests, except for the trivial. When using 88-levels/condition-names in tests, you also have the option to use SET with that 88-level to change the value stored.

COBOL is a language of fixed-length fields (unless using variable-length fields). COBOL does not have strings. If you want to know the length of the data, you have to calculate it yourself by counting the trailing spaces. FUNCTION LENGTH will tell you the length of a data-item, and you work backwards from that.

   identification division.
   program-id. hangman.

   data division.
   working-storage section.
   01 word                             pic X(100).
   01 word-length                      pic 9(3).
   01 guess                            pic X.
   01 FILLER.
      88  clear-to-no-guesses          VALUE ZERO.
      05  FILLER occurs 256 times.
          10  FILLER                   PIC X.
              88  letter-guessed       VALUE "1".
   01 FILLER                           pic X.
      88  done                         VALUE "Q".
      88  done-not                     VALUE "7".
   01 FILLER                           pic X.
      88  no-missing-letters           VALUE ":".
      88  missing-letter               VALUE "3".
   01 i                                pic 9(3).

   procedure division.
  * TODO: pick random word from word list
       move "hello"               to word
       move 5                     to word-length

  * TODO: show this in debug mode only
       display "word: " word

       set clear-to-no-guesses    TO TRUE
       set done-not               TO TRUE

       perform until done
           accept guess
           SET letter-guessed 
                ( function ord ( guess ) )
                                  TO TRUE
           SET no-missing-letters TO TRUE
           MOVE ZERO              TO i
           perform 
             word-length TIMES
               add 1              TO i
               if letter-guessed 
                   ( function ord ( word ( i : 1 ) ) ) 
                   display word ( i : 1 ) with no advancing
               else
                   SET missing-letter
                                  TO TRUE
                   display "_" with no advancing
               end-if
           end-perform
           display " "
           if no-missing-letters
               SET done           TO TRUE
           end-if
       end-perform
       goback
       .

Note I've replaced the VARYING by TIMES. There is nothing "varying" about the number of iterations for any given execution of the PERFORM, so TIMES better describes the loop. And it is faster. You have to code the initialisation and use of the subscripting item, so many people wouldn't do that. They'd go for less typing that better description.

Note the VALUE clauses on the 88s for the flags. The value does not matter (as long as they are different). The flags are only referenced in conditions and given a value by SET. In the real world, you'd use Y and N (or similar) on the VALUEs, but those values have no intrinsic meaning to the code. They are just values.

No literals with "meaning" scattered through the procedure code.

I'm not a fan of reference-modification, because it obscures. Here, you could REDEFINES word with OCCURS and use subscripting, giving the elements of word, and the data-name or index used for subscripting, a meaningful name. Not leave someone thinking "what does word ( i : 1 ) actually indicate?" (not so important here, but often is).

Note also that you use word ( i : 1 ) twice. Instead, you could MOVE word ( i : 1 ) to a new data-item, and use that subsequently.

You can write your COBOL programs as you have done, for yourself, and have fun. If you got a job writing COBOL programs, you'd be in for a bit of a change in the way you'd have to do things.

You can already start to "read" the program. A few more data-name changes, a very simple thing to do, and you'll see further readability.

Making a program readable means a programmer can understand it better. We don't expect a non-programmer to be able to make genuine sense of COBOL code. The more you simplify, the more more you describe the data, and the structure of the program, with the code, the less likely you are to confuse the programmer looking for an obscure error at 2am or wanting to implement a change to a program to timescales.

MOVE VAR1 ( i : j ) TO VAR3

or

MOVE WAREHOUSE-ROW-LOADING-BAY  
                          TO DELIVERY-BAY

The latter is COBOL, the former is... make up you own mind whether you'd like to encounter that, meaning the same thing...

\$\endgroup\$
7
\$\begingroup\$

This a very good first COBOL program. I hope you enjoy learning it. The only advice I can only offer is mostly stylistic.

I wonder whether the data types I used for the variables are the correct ones

The types are chosen are perfectly fine. In "real" code, you would proably declare numbers with a COMP clause. This means they're stored as 16/32/64-bit numbers, instead of the usual character arrays. You might also use PIC X instead of PIC A because no one uses PIC A for some reason.

I'm also worried about the hardcoded size for word.

Strings of fixed length are normal for COBOL. In fact, until COBOL 2014, strings could only have a fixed length. And, as no compiler these days targets the standard, strings will remain of fixed length for years to come.

Comments on code

In the data division:

  • almost all clauses are indented to the right side of the screen, normally column 40, for neatness
  • variables are defined in alphabetical order to make them easier to find.
  • perhaps delete state as it's not used and as it's not a very informative grouping.

Following this, your data division would become:

       data division.
       working-storage section.
       01 done                          pic 9.
       01 guess                         pic A.
       01 guesses                       occurs 256 times pic 9.
       01 i                             pic 9(3).
       01 word                          pic A(100).
       01 word-length                   pic 9(3).

In the procedure division:

  • To only display word in debug mode, you can use the >>define and >>if directives:

       >>define debug-mode
    
       >>if debug-mode is defined
           display "word: " word
       >>end-if
    
  • Instead of using perform ... until done = 1 you can use an 88-level. This is COBOL's version of a boolean variable.

       working-storage section.
       01  done-flag                     pic X.
           88  done                      value "1" false "0".
      *> ...
           perform until done
               *>  ...
               set done to true
               *> ...
                   else
                       set done to false
      *> (and similarly for guesses)
    
  • it's common style to precede indices and reference modification with a space; e.g.:

                   if guesses (function ord(word (i:1))) = 1 then
    
  • common style also dictates that the final, mandatory full stop goes on a line on its own:

           end-perform
           .
    

Finally, if you're new to COBOL, you may prefer to use fixed-format code which lets you put code in the first 7 columns and past column 72. Either compile the file with -free or preface your code with

       >>source format is free
\$\endgroup\$
  • 1
    \$\begingroup\$ 1 suggestion, if you define a variable with 88-level I would not name the field if all you use are the values. And a tip: like column 40 in the data division I find it improves readability if you line up the 'to' parts of statements in the procedure division. \$\endgroup\$ – Kwebble Apr 21 '15 at 23:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.