-2

My assignment is to use operator overloading to

  1. create a random number array
  2. get lowest number
  3. get highest number
  4. get average
  5. get total and
  6. get standard deviation.

It is just a mess. Here is my code:

#ifndef ASSIGNMENT6_HEAD6_H
#define ASSIGNMENT6_HEAD6_H

#include <iostream>
using namespace std;

class Analyzer {

    //Private Member
private:
    int numbers;

    //Public Member
public:
    Analyzer();//default constructor
    ~Analyzer();//destructor
    Analyzer operator+(const Analyzer &a) const;

    friend numbers operator+();


};//end of class

#endif //ASSIGNMENT6_HEAD6_H

//Class math with overloading operator and friends
#include "head6.h"
#include <cmath>
#include <iostream>
#include <string>
#include <iomanip>
#include <vector>
using namespace std;

vector<int> numbers;
int min = numbers[0];
int max = numbers[0];
int sizeofArray;
Analyzer::Analyzer() {

}
int getLowest(const int[], int);
//Random number member
void randNumbers(int sizeofArray, int* numbers[]) {
    for (int index = 0; index < sizeofArray; index++)
    {
        numbers[index] = (numbers() % 499) + 100;

    }return;
}
//Setters
int lowest = getLowest(numbers, sizeofArray);
int highest = getHighest(numbers, sizeofArray);
float total = getTotal(numbers);
double average = getAverage(total, sizeofArray);
//Lowest number
void getLowest(const int numbers[], int sizeofArray) {
    for (int i = 0; i < sizeofArray; i++) {
        if (min > numbers[i]) {
            min = numbers[i];
            min = lowest;
        }
    }
    return;
}
//Highest number
void getHighest(const int numbers[], int sizeofArray) {
    for (int i = 0; i < sizeofArray; i++) {
        if (max > numbers[i]) {
            max = numbers[i];
            max = lowest;
        }
    }
    return;
}
//Total
float getTotal(const int numbers) {
    total = sum(numbers[]);
    return total;
}
//Average
double getAverage(const float total, int sizeofArray) {
    double average = total / sizeofArray;
    return average;
}
//standard deviation
float  getStandardDeviation(int sizeofArray, float numbers[])const
{
    float deviation1;
    for (int i = 0; i < sizeofArray; i++)
        sum = (mean - numbers[i]) * (mean - numbers[i]);

    deviation1 = sqrt(sum / sizeofArray - 1);
    float deviation = deviation1;

    return deviation;
}

string a() {
    stringstream sout;
    sout << "STATISTICAL ANALYSIS OF RANDOMLY GENERATED NUMBERS" << endl;
    sout << "====================================================" << endl;
    sout << left << "Lowest Number:" << left << getLowest() << endl;
    sout << left << "Highest Number:" << left << getHighest() << endl;
    sout << left << "Numbers Total:" << left << getTotal() << endl;
    sout << left << "Numbers Averge:" << left << getAverage() << endl;
    sout << left << "Numbers of Standard Deviation:" << left <<
        getStandardDeviation() << endl;
    return sout.a();
}

int ​​main()
{
    Analyzer a;

    a + 100;
    cout << a;

    return 0;
}

Thank you for any assistance.

Swordfish
  • 12,971
  • 3
  • 21
  • 43
Scientist
  • 1
  • 2
  • What is the thing you pass to `randNumbers()` as 2nd parameter? – Swordfish Nov 10 '18 at 06:14
  • Please provide a [mcve]. – Swordfish Nov 10 '18 at 06:15
  • What does `numbers` look like? – Aykhan Hagverdili Nov 10 '18 at 06:16
  • everytime you use global variables, a kitty dies :( – Swordfish Nov 10 '18 at 06:27
  • + your `Analyzer operator+(const Analyzer &a) const;` takes an `Analyzer`, you pass an `int` to it which shouldn't compile. – Swordfish Nov 10 '18 at 06:29
  • whats the error message your compiler gives you for your mess? (and don't comment the obvious in your code). What does `Analyzer` need a destructor for? Don't use `using namespace` in headers. – Swordfish Nov 10 '18 at 06:30
  • Commenting code is a very good thing in general - however, don't comment the obvious (then you're just spoiling the code...). Anybody can recognise private/public members by the corresponding keyword, and anyone knows how default constructor and destructor look like, similarly the end of a class. A comment when closing a class, however, *can* be useful, if the class is large. Then, however, better mention *which* class you close: `}; // class Analizer` – Aconcagua Nov 10 '18 at 06:32
  • error C2664: 'int getLowest(const int [],int)': cannot convert argument 1 from 'std::vector>' to 'const int []' with – Scientist Nov 10 '18 at 06:33
  • @TessaJohnson `return sout.a();` with `sout` being a `std::stringstream` surely won't compile either. – Swordfish Nov 10 '18 at 06:33
  • all the functions you call in `a()` take more than zero arguments. You provide none of them. – Swordfish Nov 10 '18 at 06:37
  • "//Setters" thats not what one would consider "setters" in oop. – Swordfish Nov 10 '18 at 06:39
  • 3
    voting to close since it is unclear what you are asking. Your code has so many problems that it is allmost impossible to guess what it should be doing. Please ask specific quesitons. – Swordfish Nov 10 '18 at 06:42

2 Answers2

1

Your assignment is to use operator overloading to solve the issues - but you actually don't do so anywhere (apart from the operator+ for your Analyzer class – which is meaningless, though).

Reading your lines, I'd rather assume that you're supposed to write separate classes for each task:

class Minimum
{
    std::vector<int> const& values
    public:
    Minimum(std::vector<int> const& values) : values(values) { }
    // calculates minimum / lowest value from member:
    int operator()();
};
class Maximum
{
    public:
    //Maximum(); not needed in this variant

    // calculates maximum from parameter
    int operator()(std::vector<int> const& values);
};

void test()
{
    std::vector<int> values({10, 12, 7});
    int min = Minimum(values)();
    int max = Maximum()(values);
}

These are two different patterns, for consistency, you should select one and implement all classes alike. In first approach, you can access the vector from any member function without having to pass it around as parameter, in second approach, you can re-use one and the same object to calculate the value on several different vectors (you could still maintain a pointer to the vector to avoid passing it around via parameters...).

Coming back to your original code, unfortunately it is full of errors

vector<int> numbers;
int min = numbers[0]; // vector is yet empty! undefined behaviour!
int max = numbers[0];

Actually, you might want not to use globals at all, see later...

//int sizeofArray; // use numbers.size() instead!

// not an error, but questionable: you have a std::vector already, why do you
// fall back to C-style raw arrays?
void randNumbers(int sizeofArray, int* numbers[])
//                                   ^ array of pointers???
{
    for (int index = 0; index < sizeofArray; index++)
    {
        numbers[index] = (numbers() % 499) + 100;
        // you certainly intended to use rand function
    }
    // return; // just plain obsolete
}

// vector variant:
void randNumbers(unsigned int numberOfValues, std::vector<int>& destination)
//                            ^ not how many numbers ARE in,
//                              but how many SHALL be inserted
{
    // assuming we want to re-use this function and guarantee that EXACTLY
    // 'numberOfValues' values are contained:
    destination.clear(); // there might have been some values in already...

    // assure sufficently internal memory pre-allocated to prevent
    // multiple re-allocations during filling the vector:
    destination.reserve(numberOfValues);

    while(numberOfValues--)
    {
        numbers.push_back(rand() * 500 / RAND_MAX + 100);
        // modulus is unprecise; this calculation will give you better
        // distribution
        // however, rather prefer modern C++ random number generators!
        // IF you use rand: assure that you call srand, too, but exactly ONCE,
        // best right when entering main function
    }
}

// C++ random number generator:
void randNumbers(unsigned int numberOfValues, std::vector<int>& destination)
{
    static std::uniform_int_distribution<> d(100, 599);
    static std::mt19937 g;

    destination.clear();
    destination.reserve(numberOfValues);
    while(numberOfValues--)
    {
        numbers.push_back(d(g));
    }
}

Now you have contradicting function declarations:

int  getLowest(const int[], int);
void getLowest(const int numbers[], int sizeofArray) { /* ... */ }

int lowest = getLowest(numbers, sizeofArray);
// again: the vector is yet empty!
// so you certainly won't get the result desired
// however, this won't compile at all: numbers is a std::vector,
// but parameter type is array, so you need:
int lowest = getLowest(numbers.data(), numbers.size());
//                                     ^ replaced the redundant global as well
// move this into your main function AFTER having filled the vector!

// picking int as return value:
int getLowest(const int numbers[], unsigned int sizeofArray)
{
    // you'd now have to initialize the global first; better, though:
    // return a local variable:

    // this assumes that there is at least one element in! check before usage
    // and decide what would be the appropriate error handling if the vector
    // is empty (return 0? return INT_MIN? throw an execption?)
    int min = numbers[0];

    for (int i = 1; i < sizeofArray; i++)
    {
        if (min > numbers[i])
        {
            min = numbers[i];
            // min = lowest; // don't overwrite the minimum again!
        }
    }

    // returning at end of void function is obsolete, don't do that explicitly
    // well, with int as return value, as is NOW, you NEED to return:
    return min;
}

Maximum analogously, be aware that you did not change the comparison from > to <! Be aware that there are already std::min_element, std::max_element and std::minmax_element which do the same (if not prohibited by the assignment, you should rather use these instead of re-inventing the wheel).

// prefere double! float (on typical machines at least) has same size as int
// and it is quite likely that you will lose precision due to rounding; I
// personally would rather use int64_t instead, so you won't run into rounding
// issues even with double and you'd need quite a large amount of summands
// before overflow can occur...
float getTotal(const int numbers) // just one single number???
{
    total = sum(numbers[]);
    // index operator cannot be applied on a single int; additionally, you need
    // to provide an argument; where is 'sum' function defined at all???

    return total;
}

// prefer double again
double getStandardDeviation(int sizeofArray, float numbers[]) // const
// (free standing functions cannot be const)
{
    // mean isn't declared/defined anywhere (average instead?)!
    // and you need to declare and initialize the sum appropriately:
    double sum = 0.0;

    float deviation1;
    for (int i = 0; i < sizeofArray; i++)
        sum += (mean - numbers[i]) * (mean - numbers[i]);
    //      ^ you need to add, if you want to build sum

    // why two variables, even both of same type???
    deviation1 = sqrt(sum / sizeofArray - 1);
    float deviation = deviation1;

    return deviation;

    // simplest: drop both deviation and deviation 1 and just do:
    return sqrt(sum / sizeofArray - 1);
}

Finally: I don't think that you'd use the resulting string (below) for anything else than printing out to console again, so I'd output to std::cout directly (naming the function 'print'); if at all, I'd provide a std::ostream as parameter to be more flexible:

void print(std::ostream& sout)
{
    sout << "STATISTICAL ANALYSIS OF RANDOMLY GENERATED NUMBERS" << endl;
    sout << "====================================================" << endl;
    sout << left << "Lowest Number:" << left << getLowest() << endl;
    sout << left << "Highest Number:" << left << getHighest() << endl;
    sout << left << "Numbers Total:" << left << getTotal() << endl;
    sout << left << "Numbers Averge:" << left << getAverage() << endl;
    sout << left << "Numbers of Standard Deviation:" << left
         << getStandardDeviation() << endl;
}

Now you could pass std::cout to, a std::ostringstream object or even write to file via a std::ofstream...

int ​​main()
{
    Analyzer a, b, c; // b, c added by me for illustration only

    a + 100;
    // the operator accepts another Analyzer object, so you could do
    c = a + b;

    cout << a; // there's no operator<< overload for Analyzer class


    // it is HERE where you'd call all your getXZY functions!

    return 0;
}
Aconcagua
  • 24,880
  • 4
  • 34
  • 59
0

You are passing a pointer to an array of integers:

void randNumbers(int sizeofArray, int* numbers[])

where you really just want to pass numbers as an array. And since all arrays degrade to pointers when passed as a parameter, your function is simply this:

void randNumbers(int sizeofArray, int* numbers) {

    for(int index = 0; index < sizeofArray; index++) {
        numbers[index]= (rand() % 499) + 100;
    };

}

The result is that the items in numbers will be integers in the range of [100..599] inclusive.

selbie
  • 100,020
  • 15
  • 103
  • 173
  • I personally prefer `int numbers[]` syntax in given case, makes more obvious that really an array is expected... – Aconcagua Nov 10 '18 at 06:35
  • @Aconcagua I detest the usage of `[]` when it is used to "hide" a pointer. – Swordfish Nov 10 '18 at 06:38
  • @Swordfish Fine (not meant ironically!), now we are complete: pro (distinguish array from simple pointer, e. g. as additional return value) and contra (hiding pointer nature). Now up to the reader which one he/she wants to weigh stronger... – Aconcagua Nov 10 '18 at 07:10
  • @Aconcagua i don't get your point. – Swordfish Nov 10 '18 at 07:13
  • @Swordfish `int set(int* target) { target = 7; } void iterate(unsigned int num, int[] values) { while(num--) ++values; }`. If applied consistently (I do so with exception of C-strings), the syntax difference can be used to indicate which kind of pointer is expected (to single element or to first of array), additionally a hint what the function might do. Maybe marginal, but still... – Aconcagua Nov 10 '18 at 07:42
  • @Swordfish A step further: [`int pipe(int fd[2])`](https://linux.die.net/man/2/pipe); sure, the size is ignored, we still have a just a pointer, but we tell at least what (minimum) size we expect... – Aconcagua Nov 10 '18 at 07:47
  • @Aconcagua a matter of taste, i quess. – Swordfish Nov 10 '18 at 07:53
  • @Swordfish Certainly - that's why I initially started with "I personaly [...]". Still we got a nice little discussion about pros and cons, might help someone to make up his/her own mind... – Aconcagua Nov 10 '18 at 08:29