Like Tree1Likes
  • 1 Post By laserlight

Just looking for Feedback

This is a discussion on Just looking for Feedback within the C++ Programming forums, part of the General Programming Boards category; Here was the problem I was attempting to solve with this program. Program: Write a program that takes in 50 ...

  1. #1
    Registered User
    Join Date
    Jun 2012
    Posts
    19

    Post Just looking for Feedback

    Here was the problem I was attempting to solve with this program. Program: Write a program that takes in 50 values and prints out the highest, the lowest, the average and then all 50 input values, one per line.

    Below is my solution, just wondering if you all see any errors, or if you would have done anything different... Thank you for your time!

    Note: I have only gotten up to the complexity of arrays, I am still learning so if this seems basic its probably because I don't know a more complex/easier method as of yet. This was a problem in the "arrays" chapter of the book.

    Code:
    #include <cstdlib>
    #include <ctime>
    #include <iostream>
    
    
    using namespace std;
    
    
    int array[50];
    int highestValue = array[0];
    int lowestValue = array[0];
    int findHighest (int array[], int size)
    {
        for (int i = 0; i < size; i++)
        {
            if ( array[ i ] > highestValue)
            {
                highestValue = array[i];
            }
        }
        return highestValue;
    }
    int findLowest (int array[], int size)
    {
        for (int i = size; i > 1; i--)
        {
            if ( array[ i ] < lowestValue)
            {
                lowestValue = array[i];
            }
        }
        return lowestValue;
    }
    int findAverage (int array[], int size)
    {
        int sum = 0;
        for (int i = 0; i < size; i++)
        {
            sum += array[i];
        }
        return sum/50;
    }
    void displayArray (int array[], int size)
    {
        cout << "{";
        for (int i = 0; i < size; i++)
        {
            if ( i != 0 )
            {
                cout << "\n";
            }
            cout << array [i];
        }
        cout << "}";
    }
    
    
    int main ()
    {
        srand(time(NULL));
        for ( int i = 0; i < 50; i++ )
        {
            array[i] = rand() % 100;
        }
        cout << "Highest Value: " << findHighest (array,50) <<"\n";
        cout << "Lowest Value: " << findLowest (array,50) <<"\n";
        cout << "Average Value: " << findAverage (array,50) <<"\n";
        cout << "Array: ";
        displayArray(array, 50);
        cout << "\n";
        return 0;
    }

  2. #2
    Registered User
    Join Date
    May 2009
    Posts
    2,507
    You initialize highestValue and lowestValue to a junk value in the first position of the array.

    Note: Having highestValue and lowestValue as Global Variables is also poor style/design.

    Note: Having array being global might be considered OK; but, I would NOT make it global in most cases.

    Code:
    int array[50];
    int highestValue = array[0];
    int lowestValue = array[0];
    Tim S.
    Last edited by stahta01; 07-06-2012 at 10:27 AM.
    "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the universe trying to produce bigger and better idiots. So far, the Universe is winning." Rick Cook

  3. #3
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,407
    I notice that you have three global variables:
    Code:
    int array[50];
    int highestValue = array[0];
    int lowestValue = array[0];
    array should be local to the main function, then passed around as needed. highestValue and lowestValue should be local to findHighest and findLowest, respectively.

    Also, since you have the using directive using namespace std; you should not use the name array as it may conflict with std::array. An alternative name like numbers could work.
    stahta01 likes this.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #4
    Registered User manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    Kolkata@India
    Posts
    2,498
    Also, you are doing in different loops, what you can easily do in a single iteration.
    That may not look so structured in the form of functions.. but it sure works better.
    Manasij Mukherjee | gcc-4.8.2 @Arch Linux
    Slow and Steady wins the race... if and only if :
    1.None of the other participants are fast and steady.
    2.The fast and unsteady suddenly falls asleep while running !



  5. #5
    Registered User
    Join Date
    May 2009
    Posts
    2,507
    The function findAverage has the magic number 50 in it; this is poor style and design.

    http://en.wikipedia.org/wiki/Magic_n...ical_constants

    Tim S.
    Last edited by stahta01; 07-06-2012 at 10:50 AM. Reason: added url
    "Programming today is a race between software engineers striving to build bigger and better idiot-proof programs, and the universe trying to produce bigger and better idiots. So far, the Universe is winning." Rick Cook

  6. #6
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,422
    The standard library already have algorithms for finding max and min in a set of elements already, actually. You may want to try getting familiar with them as an exercise next.
    They're called min_element and max_element, found in <algorithm>.
    STL Algorithms - C++ Reference
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  7. #7
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,293
    If it were me, the number 50 would only appear in one place in the entire program.
    There are various ways to ensure that, just pick one that works for you.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  8. #8
    Registered User
    Join Date
    Jun 2005
    Posts
    6,197
    Apart from things already mentioned (not using statics, using standard algorithms, avoiding magic values) I also wouldn't even use an array. I would employ one of the standard containers. I'd also make use of a stream inserter for the output.

    Doing all those things would give code that almost certain works better, is much briefer, easier to understand, and easier to maintain.

    Personally, I avoid "using namespace" directives as well, but that is as much a stylistic choice as it is a technical one.
    Right 98% of the time, and don't care about the other 3%.

  9. #9
    - - - - - - - - oogabooga's Avatar
    Join Date
    Jan 2008
    Posts
    2,808
    I'm not sure why you're indexing the array backwards in findLowest, but you access array[size], which is out-of-bounds. Should be:
    Code:
    int findLowest (int array[], int size)
    {
        int lowest = array[0];
        for (int i = 1; i < size; i++)
            if (array[ i ] < lowest)
                lowest = array[i];
        return lowest;
    }
    The cost of software maintenance increases with the square of the programmer's creativity. - Robert D. Bliss

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Looking For Feedback
    By Matty_Alan in forum C++ Programming
    Replies: 2
    Last Post: 05-27-2010, 04:33 PM
  2. K&R2, 1-24. Feedback
    By Tool in forum C Programming
    Replies: 0
    Last Post: 11-14-2009, 09:49 AM
  3. Feedback?
    By CornedBee in forum A Brief History of Cprogramming.com
    Replies: 8
    Last Post: 01-02-2004, 10:01 PM
  4. feedback, please...
    By major_small in forum C++ Programming
    Replies: 5
    Last Post: 08-24-2003, 08:17 PM
  5. Looking for some feedback...
    By dead_cell in forum C++ Programming
    Replies: 7
    Last Post: 08-11-2002, 09:08 AM

Tags for this Thread


1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21