Re: one more

Discussion in 'C++' started by rossum, Oct 18, 2004.

  1. rossum

    rossum Guest

    On 17 Oct 2004 03:22:03 -0700, (sudeep)
    wrote:

    >Implementation of BigInt
    >Problem Statement
    >

    [snip]

    Sudeep,

    You don't say what your problem is, but I think that I can see some of
    it. The code your instructor supplied does not look very good at all.

    >#include <iostream.h>

    Warning: iostream.h is deprecated in the standard and should not be
    used for new code. Use iostream (no ".h") instead.

    >#include "BigInt.h"

    Style: Put this above the iostream include to be sure that there is no
    hidden dependency on iostream in BigInt.h

    >#define SIZE 5

    Warning: As the FAQ says, "Macros are evil". Why use a global macro,
    which completely ignores type and scope, when a constant would be much
    safer?

    >
    >BigInt big_array[SIZE], minint, maxint;

    Warning: Global variables are dangerous. Why use them when they are
    not absolutely required?
    Style: Better to have one declaration per line.

    >
    >int main() {
    >
    > BigInt sum(0); // Constructor with integer parameter
    > BigInt product("1"); // Constructor with string
    > int i ;

    Style: i is only used as a loop variable, better to declare it inside
    the for statement.

    >
    > for (i=0; i<SIZE; i++) {

    Style: I would use more spaces to make this line more readable.
    Style: ++i is never slower and may be faster than i++; same for --i
    and i--. Where either would do it is better to get into the habit of
    using ++i.

    > cin >> big_array; // read BIGINT from stdin

    Style: The comment needs updating to reflect the actual code.

    > if (0==i) {
    > maxint = big_array); // assignment

    Error: Unmatched closing bracket near the end. This code will not
    compile as written.

    > minint = big_array);

    Error: Another unmatched closing bracket.

    > }
    > else {
    > if (maxint < big_array) // relational operation

    Style: Using an else if construction would reduce the amount of
    indentation needed here.

    > maxint = big_array);

    Error: A third unmatched closing bracket.

    > else if (minint > big_array)
    > minint = big_array);

    Error: A fourth unmatched bracket.

    > }
    > }
    > sum = maxint + minint;
    > product = big_array[1] * big_array[4];
    >
    > cout << sum << endl ;

    Style: There is no on-screen indication of what number this is, better
    to tell the user what it is.

    > cout << maxint - minint << endl ;
    > cout << product << endl ;
    > cout << big_array[3] / big_array[0] << endl ;
    > cout << big_array[3] % big_array[0] << endl ;


    Style: I know that "return" is not required at the end of main(), but
    I put it in anyway. This helps by showing that I have finished coding
    and hopefully not left anything off the end.

    >}


    Looking at the list of deliverables there is a possible clue to some
    of the bad style in the example: "2. BigInt.c". Note that is
    "BigInt.c", not BigInt.cpp. This has possibly been converted from an
    original in C rather than written from scratch in C++.

    A better version of the example code would be:

    #include "BigInt.h"
    #include <iostream>

    int main() {
    using std::cout;
    using std::endl;

    const int size = 5;

    BigInt big_array[size];
    BigInt minint;
    BigInt maxint;

    BigInt sum(0); // Constructor with integer parameter
    BigInt product("1"); // Constructor with string

    for (int i = 0; i < size; ++i) {
    std::cin >> big_array; // read BigInt from cin
    if (0 == i) {
    maxint = big_array; // assignment
    minint = big_array;
    }
    else if (maxint < big_array) { // relational operation
    maxint = big_array;
    }
    else if (minint > big_array) {
    minint = big_array;
    }
    }

    sum = maxint + minint;
    product = big_array[1] * big_array[4];

    cout << "Sum = " << sum << endl ;
    cout << "Difference = " << maxint - minint << endl ;
    cout << "Product = " << product << endl ;
    cout << "Division = " << big_array[3] / big_array[0] << endl ;
    cout << "Modulus = " << big_array[3] % big_array[0] << endl ;

    return 0;
    }

    If you get any problems with the rest of the assignment then post what
    you have written together with an explanation of your problems. The
    more of your own work you show, the more likely you are to get help.


    rossum


    --

    The ultimate truth is that there is no Ultimate Truth
     
    rossum, Oct 18, 2004
    #1
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Michael
    Replies:
    4
    Views:
    473
    Matt Hammond
    Jun 26, 2006
  2. Merciadri Luca
    Replies:
    4
    Views:
    853
  3. Robert Klemme

    With a Ruby Yell: more, more more!

    Robert Klemme, Sep 28, 2005, in forum: Ruby
    Replies:
    5
    Views:
    242
    Jeff Wood
    Sep 29, 2005
  4. Steven D'Aprano
    Replies:
    0
    Views:
    146
    Steven D'Aprano
    Dec 23, 2013
  5. Replies:
    3
    Views:
    119
    Gary Herron
    Dec 23, 2013
Loading...

Share This Page