HiveBrain v1.2.0
Get Started
← Back to all entries
patternModerate

Circumference and Area of a Circle in COBOL

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
circlecircumferenceandareacobol

Problem

I recently started Learning COBOL from Lynda and this is my code for the first challenge .

The Objective is that the user will provide the radius and we have to calculate the Circumference and the area of the circle and then display it.

Here is the Code.

IDENTIFICATION DIVISION.
   PROGRAM-ID. CIRCUMFERENCE AS "CIRCUMFERENCE".
   AUTHOR. ASWIN MOHAN.

  ***************************************************************
  *    Create a Program to read the Radius of a circle and THEN
  *    print out the Circumference and the radius of the circle.
  ***************************************************************

   ENVIRONMENT DIVISION.

   CONFIGURATION SECTION.
   SOURCE-COMPUTER.
   OBJECT-COMPUTER.

   INPUT-OUTPUT SECTION.

   DATA DIVISION.

   FILE SECTION.

   WORKING-STORAGE SECTION.
    01   WS-CONSTANTS.
       05    WS-PI                 PIC 9V9999  VALUE 3.1415.
    01   WS-CIRCLE.
       05    WS-RADIUS             PIC 999V99 VALUE ZEROES.
       05    WS-CIRCUMFERENCE      PIC 9999V99 VALUE ZEROES.
       05    WS-AREA               PIC 99999V99 VALUE ZEROES.
    01   WS-DISPLAY.
       05    WS-DISPLAY-CIRCUMFERECE   PIC ZZ99.99.
       05    WS-DISPLAY-AREA           PIC ZZ999.99.

   PROCEDURE DIVISION.
    0100-PROCESS-RECORDS.

        DISPLAY "Enter the Radius".
        ACCEPT WS-RADIUS.

        COMPUTE WS-CIRCUMFERENCE = 2 * WS-PI * WS-RADIUS.
        MOVE WS-CIRCUMFERENCE TO WS-DISPLAY-CIRCUMFERECE.

        COMPUTE WS-AREA = WS-PI * WS-RADIUS * WS-RADIUS.
        MOVE WS-AREA TO WS-DISPLAY-AREA.

        DISPLAY "The Circumferece is " , WS-DISPLAY-CIRCUMFERECE.
        DISPLAY "The Area is " , WS-DISPLAY-AREA.

        STOP RUN.

        END PROGRAM CIRCUMFERENCE.


I'm using GNU-COBOL and I'm worried that my COBOL coding style is not so great. I'm just starting out so all feedback is appreciated.

Solution

Is Lynda that woman from LinkedIn, or just some mate of yours? I wonder how good a teacher she is, and what her intent is. Is it to learn COBOL, or GnuCOBOL.

The reason I say that is that COBOL has Standards. The latest is 2014, which supersedes 2002, then there's 1985 and 1974. There is a 1968 Standard, but it is extremely unlikely that any current compiler in use is to that Standard.

There are current compilers to the 1974 Standard. However, I'd not recommend that as a lowest common denominator, because the 1985 Standard introduced so much that eases coding and use of COBOL.

If learning COBOL (or being taught COBOL) with the intention of using it in a paid job, I'd recommend sticking to the 1985 Standard.

There are also Language Extensions. Everyone has them. Sometimes they coincided, sometimes they don't. GnuCOBOL supports many Extensions from other compilers. However, again, if the aim is to learn COBOL to get a job, beware of the casual use of Extensions.

An area where there are an absolute plethora of Extensions (to 1985) is in human interaction. DISPLAY and ACCEPT can be entirely different beasts from compiler to compiler. To use them in a portable way, use their very simplest forms.

This is sometimes not easy, because although your PROCEDURE DIVISION use is simple, you are, unknowingly, using a GnuCOBOL extension which provides conversion when the target of ACCEPT is a numeric value. If you were to work on an IBM COBOL, the same code would not be reliable.

I agree with Bruce and Edward in their answers. Do something about how your program looks. Exactly what you do in a personal sense depends. Be aware that if you get a COBOL job, you will likely be working in a team, and there will likely be local standards for how your program looks, and how things are done. This eases the ability to pick up someone else's program. You will be able to suggest changes for "bad" practices, but if you can't win the argument, do what the others do at your site.

Next, not so much a COBOL thing as a "programming" thing, I hope, although there are COBOL specifics affecting it. Your program doesn't work.

You can take from the user a maximum value of 999.99 for a radius. Your definitions, specifically for the area, do not allow for a radius of that size.

In COBOL you have to think longer about such things, because definitions, except when they are not, are fixed in size. The size you have for your fields containing "area" are orders of magnitude off. The maximum area is over three million for your maximum radius, but you only allow for five integer digits in your definitions.

Edward suggests using a more accurate value for PI. This is not something to be done blindly. The precision of PI to use should be defined in the specification for the program, as should any and all other limits and required precision. You can't write a COBOL program and then say to the Accountant "well, it works the way I've defined it".

Readability of output is also important. Use separators for large numbers, and zero-suppress down to the single digit before the decimal point. Unless your specification says otherwise. Line the numbers in the output on the decimal point. Much easier to consume as a human reader. Or switch the output to columnar with headings. Depends on the spec.

Be careful of using VALUE clauses unnecessarily. All of your VALUE ZEROES (which I'd code as "VALUE ZERO", but what the heck) are unnecessary. All of those fields are targets before they are sources, so the value of zero will never be relevant to anything. Having the VALUE implies that the value has a significance, which is misleading when it does not. Don't worry, very few programmers actually do that :-)

Indentation is for meaning. If the indentation makes the meaning clearer, do it. If not, don't. There is no point that I can discern in indenting 01-levels beyond what is necessary, or paragraph/SECTION-names.

No one mentioned the commas. Don't use commas (except in literals). They have no meaning. Don't use semi-colons. They have no meaning. To confirm this, add this line anywhere in the PROCEDURE DIVISION and recompile.

,,,,;;; , , , ; ; ;


Still compile as before? Still run as before? What is the point then. Ok, how do you make it easier for a human to read when there are multiple targets/sources:

ADD A
            B
            C                    TO D
                                    E 
                                    F


The names are just for examples, and you should always use meaningful names, for everything. It helps the source code tell the story of the program.

If you are going to use comments, make sure they are accurate. Comments are never compiled, so are only ever "checked" by the human reader. Your comments have significant errors and misinformation. Don't do that. If you then find there is little to say in a comment, because you have structured your program, and used good names, feel free not to use many co

Code Snippets

,,,,;;; , , , ; ; ;
ADD A
            B
            C                    TO D
                                    E 
                                    F
IDENTIFICATION DIVISION.
   PROGRAM-ID. CIRCUMF.
  *    Obtain the Radius of a circle and THEN
  *    print out the Circumference and the area of the circle.
   DATA DIVISION.
   WORKING-STORAGE SECTION.
   01  WS-PI                               PIC 9V9999  
                                            VALUE 3.1415.
   01  WS-CIRCLE.
       05  WS-RADIUS                       PIC 999V99.
       05  WS-CIRCUMFERENCE                PIC 9(4)V99.
       05  WS-AREA                         PIC 9(7)V99.
   01  WS-DISPLAY.
       05  WS-DISPLAY-CIRCUMFERECE         PIC Z,ZZZ,ZZ9.99.
       05  WS-DISPLAY-AREA                 PIC Z,ZZZ,ZZ9.99.
   PROCEDURE DIVISION.
       DISPLAY 
               "Enter the Radius"
       ACCEPT 
              WS-RADIUS
       COMPUTE WS-CIRCUMFERENCE     = 2 * WS-PI 
                                      * WS-RADIUS 
       MOVE WS-CIRCUMFERENCE        TO WS-DISPLAY-CIRCUMFERECE
       COMPUTE WS-AREA              = ( WS-PI 
                                        * ( WS-RADIUS 
                                          * WS-RADIUS ) )
       MOVE WS-AREA                 TO WS-DISPLAY-AREA
       DISPLAY 
               "The Circumferece is "  
               WS-DISPLAY-CIRCUMFERECE
       DISPLAY 
               "The Area is         " 
               WS-DISPLAY-AREA
       STOP RUN
       .
05  WS-RADIUS-OVERSIZE              PIC 9(4)V99.
       05  FILLER 
           REDEFINES WS-RADIUS-OVERSIZE.
           10  FILLER                      PIC X.
           10  WS-RADIUS                   PIC 999V99.
ACCEPT WS-RADIUS-OVERSIZE

Context

StackExchange Code Review Q#128868, answer score: 12

Revisions (0)

No revisions yet.