Thread: Just looking for Feedback

  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
    4,183
    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.
    "...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson

  3. #3
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    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.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  4. #4
    [](){}(); manasij7479's Avatar
    Join Date
    Feb 2011
    Location
    *nullptr
    Posts
    2,657
    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.

  5. #5
    Registered User
    Join Date
    May 2009
    Posts
    4,183
    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
    "...a computer is a stupid machine with the ability to do incredibly smart things, while computer programmers are smart people with the ability to do incredibly stupid things. They are,in short, a perfect match.." Bill Bryson

  6. #6
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    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,318
    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,815
    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%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  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, 10:49 AM
  3. Feedback?
    By CornedBee in forum A Brief History of Cprogramming.com
    Replies: 8
    Last Post: 01-02-2004, 11: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