My numeric analyzer

Please tell me your ideas about the program.

Code:

---

// By SiavoshKC
//
//This program reads numbers from a text file and analyses them.
//It doesn't use vector maybe for simplfication (?)
#include "stdafx.h"

//This class manages every thing in this program

class Numbers{
public:
        Numbers(char name[100]);

        long long Sum();
        int Ave();
        int Mid();
        void PrintNums();
        int Min();
        int Max();
        int Count(){
                return numCount;
        }
        ~Numbers()
        {
                if(nums != NULL) delete[] nums;
                if(fileOfNumbers != NULL) delete fileOfNumbers;

        }

private:
        unsigned int numCount;
        void ReadFile();
        enum errTypeE {FILE_NOT_OPENED=1, ERR_READING_FILE, MEM_ALLOC_FAILED};
        void ErrHandling(errTypeE err);
        char fileName[100];
        ifstream *fileOfNumbers;
        int *nums;
       

};

Numbers::Numbers(char name[100])
{
        nums = NULL;
        strcpy_s(fileName, name);
        fileOfNumbers = new ifstream(fileName);
        if(!fileOfNumbers->good()) ErrHandling(FILE_NOT_OPENED);
        ReadFile();
        fileOfNumbers->close();
}

void Numbers::ReadFile()
{
        int temp;
        numCount = 0;
        //This loop counts the items in the text file
        while(!fileOfNumbers->eof()){
                cin.clear();
                *fileOfNumbers >> temp ;
                if(!fileOfNumbers->bad())numCount++;
                else ErrHandling(ERR_READING_FILE);
        }
        try {
                nums = new int[numCount+1];
        }
        catch (...) {
                ErrHandling(MEM_ALLOC_FAILED);
        }

        numCount = 0;
        fileOfNumbers->seekg(0);
        fileOfNumbers->clear();
        //Now saving numbers in memory
        while(!fileOfNumbers->eof()){
                cin.clear();        //Bad style?
                *fileOfNumbers >> nums[numCount] ;
                if(!fileOfNumbers->bad())numCount++;
                else ErrHandling(ERR_READING_FILE);
        }


}

void Numbers::PrintNums()
{
        for(int i = 0; i<numCount;i++) cout << nums[i] << endl;
}

void Numbers::ErrHandling(errTypeE err)
{
        switch (err) {
case FILE_NOT_OPENED:
                cerr << "ERROR: File could not be opened!"<<endl;
                this->~Numbers();
                exit(err);
                break;

case ERR_READING_FILE:
                cerr << "ERROR: File can not be read!" << endl;
                this->~Numbers();
                exit(err);
                break;
case MEM_ALLOC_FAILED:
                cerr << "ERROR: Memory allocation failed!" << endl;
                exit(err);
                break;
        }
}

int Numbers::Ave()
{
        return(Sum()/numCount);
}

long long Numbers::Sum()
{
        return accumulate(nums,nums+numCount,0);
}

int Numbers::Max()
{
        return *max_element(nums,nums+numCount);
}

int Numbers::Min()
{
        return *min_element(nums,nums+numCount);
}

int Numbers::Mid()
{
        sort(nums, nums + numCount);
        return (nums[numCount/2]);
}


int _tmain(int argc, _TCHAR* argv[])
{
        Numbers nums("num.txt");
        nums.PrintNums();
        cout << "-------------------------------------------------------------------" << endl;
        cout << "Count: " << nums.Count() << endl;
        cout << "Sum: " << nums.Sum() << endl;
        cout << "Average: " << nums.Ave() << endl;
        cout << "Mid: " << nums.Mid() << endl;
        cout << "Max: " << nums.Max() << endl;
        cout << "Min: " << nums.Min() << endl;
        cout << "\nPress a key to exit..." << endl;
        getch();
        return 0;
}

---

stdafx.h

Code:

---

// stdafx.h : include file for standard system include files,
// or project specific include files that are used frequently, but
// are changed infrequently
//

#pragma once


#define WIN32_LEAN_AND_MEAN                // Exclude rarely-used stuff from Windows headers
#include <cstdio>
#include <tchar.h>
#include <fstream>
#include <iostream>
#include <numeric>
#include <algorithm>
#include <cstdlib>
#include <conio.h>

using namespace std;



// TODO: reference additional headers your program requires here

---

This is not a good idea

Code:

---

exit(err);

---

The program will just exit no destructors will be called.

Code:

---

        while(!fileOfNumbers->eof()){
                cin.clear();        //Bad style?
                *fileOfNumbers >> nums[numCount] ;
                if(!fileOfNumbers->bad())numCount++;
                else ErrHandling(ERR_READING_FILE);
        }

---

Not a good idea either. nums will contain the last number of the file twice.
And I wonder what the cin.clear() is supposed to do.
Kurt

Quote:

---

This is not a good idea

Code:
exit(err);

---

Why? I called the destructor myself before exit().

Quote:

---

nums will contain the last number of the file twice.
And I wonder what the cin.clear() is supposed to do.

---

Without cin.clear() it doesn't work. I don't know exactly why. You know it is one year that I didn't program, I've forgotten many things.

Although nums keep one extra number, numCount is correct.

Quote:

---

Originally Posted by siavoshkc

Why? I called the destructor myself before exit().

---

What about any other objects that your program uses ?
What if that object of Numbers was dynamically allocated ?
Kurt

OK, I changed it, I will correct that exit as well.

Code:

---

void Numbers::ReadFile()
{
        int temp;
        numCount = 0;
        //This loop counts the items in the text file
        while(*fileOfNumbers >> temp){
                if(!fileOfNumbers->bad())numCount++;
                else ErrHandling(ERR_READING_FILE);
        }
        try {
                nums = new int[numCount+1];
        }
        catch (...) {
                ErrHandling(MEM_ALLOC_FAILED);
        }

        numCount = 0;
        fileOfNumbers->clear();
        fileOfNumbers->seekg(0);
       
        //Now saving numbers in memory
        while(*fileOfNumbers >> nums[numCount]){
                if(!fileOfNumbers->bad())numCount++;
                else ErrHandling(ERR_READING_FILE);
        }


}

---

Code:

---

        while(*fileOfNumbers >> temp){
                if(!fileOfNumbers->bad())numCount++;

---

This doesn't make sense.
if the stream is bad that statement will never be reached.
Kurt

And it is the last:

Code:

---

// By SiavoshKC
//
//This program reads numbers from a text file and analyses them.
//It doesn't use vector maybe for simplfication (?)
#include "stdafx.h"



enum errTypeE {FILE_NOT_OPENED=1, ERR_READING_FILE, MEM_ALLOC_FAILED};

class NumbersException{
public:
        NumbersException(errTypeE err){
                thisErr = err;

        }
        NumbersException(errTypeE err, char fileName[100]){
                thisErr = err;
                strcpy_s(fName, fileName);

        }
        char fName[100];
        errTypeE thisErr;
        void PrintErrDescription(){
                switch (thisErr) {
        case FILE_NOT_OPENED:
                        cerr << "ERROR: File " << fName <<" could not be opened!"<<endl;
                        break;

        case ERR_READING_FILE:
                        cerr << "ERROR: File " << fName <<" can not be read!" << endl;
                        break;
        case MEM_ALLOC_FAILED:
                        cerr << "ERROR: Memory allocation failed!" << endl;
                        break;
                }

        }
};

class Numbers{
public:
        Numbers(char name[100]);

        long long Sum();
        int Ave();
        int Mid();
        void PrintNums();
        int Min();
        int Max();
        bool IsOK() { return isOK;}
        int Count(){
                return numCount;
        }
        ~Numbers()
        {
                if(nums != NULL) delete[] nums;
                if(fileOfNumbers != NULL) delete fileOfNumbers;

        }
       
private:
        bool isOK;
        unsigned int numCount;
        void ReadFile(char name[100]);
       
        void ErrHandling(errTypeE err);
        char fileName[100];
        ifstream *fileOfNumbers;
        int *nums;
       

};

Numbers::Numbers(char name[100])
{
        isOK = 1;
        nums = NULL;
        try{
                ReadFile(name);
        }catch(NumbersException *err){
                isOK = 0;
                err->PrintErrDescription();
        }

       
}

void Numbers::ReadFile(char name[100])
{
        strcpy_s(fileName, name);
        fileOfNumbers = new ifstream(fileName);
        if(!fileOfNumbers->good()) throw(new NumbersException(FILE_NOT_OPENED, fileName));

        int temp;
        numCount = 0;
        //This loop counts the items in the text file
        while(*fileOfNumbers >> temp){
                numCount++;
        }
        try {
                nums = new int[numCount+1];
        }
        catch (...) {
                throw (new NumbersException(MEM_ALLOC_FAILED));
        }

        numCount = 0;
        fileOfNumbers->clear();
        fileOfNumbers->seekg(0);
       
        //Now saving numbers in memory
        while(*fileOfNumbers >> nums[numCount]){
                numCount++;
        }
        fileOfNumbers->close();

}

void Numbers::PrintNums()
{
        for(int i = 0; i < numCount;i++) cout << nums[i] << endl;
}



int Numbers::Ave()
{
        return(Sum()/numCount);
}

long long Numbers::Sum()
{
        return accumulate(nums,nums+numCount,0);
}

int Numbers::Max()
{
        return *max_element(nums,nums+numCount);
}

int Numbers::Min()
{
        return *min_element(nums,nums+numCount);
}

int Numbers::Mid()
{
        sort(nums, nums + numCount);
        return (nums[numCount/2]);
}


int _tmain(int argc, _TCHAR* argv[])
{
        Numbers nums("C:\\numf.txt");
        if(nums.IsOK()) {
                nums.PrintNums();
                cout << "-------------------------------------------------------------------" << endl;
                cout << "Count: " << nums.Count() << endl;
                cout << "Sum: " << nums.Sum() << endl;
                cout << "Average: " << nums.Ave() << endl;
                cout << "Mid: " << nums.Mid() << endl;
                cout << "Max: " << nums.Max() << endl;
                cout << "Min: " << nums.Min() << endl;
               
        }
        else
                cout << "There was an unrecoverable error. Sorry for inconvenience!" << endl;
       
        cout << "\nPress a key to exit..." << endl;
        _getch();
        return 0;
}

---

I edited my last.

With minor changes:

Code:

---

// By SiavoshKC
//
//This program reads numbers from a text file and analyses them.
//It doesn't use vector maybe for simplfication (?)
#include "stdafx.h"



enum errTypeE {FILE_NOT_OPENED=1, ERR_READING_FILE, MEM_ALLOC_FAILED};

class NumbersException{
public:
        NumbersException(errTypeE err){
                thisErr = err;

        }
        NumbersException(errTypeE err, char fileName[100]){
                thisErr = err;
                strcpy_s(fName, fileName);

        }
        char fName[100];
        errTypeE thisErr;
        void PrintErrDescription(){
                switch (thisErr) {
        case FILE_NOT_OPENED:
                        cerr << "ERROR: File " << fName <<" could not be opened!"<<endl;
                        break;

        case ERR_READING_FILE:
                        cerr << "ERROR: File " << fName <<" can not be read!" << endl;
                        break;
        case MEM_ALLOC_FAILED:
                        cerr << "ERROR: Memory allocation failed!" << endl;
                        break;
                }
        }
};


class Numbers{
public:
        Numbers(char name[100]);

        long long Sum();
        int Ave();
        int Mid();
        void PrintNums();
        int Min();
        int Max();
        int Mod();
        bool IsOK() { return isOK;}
        int Count(){
                return numCount;
        }
        ~Numbers()
        {
                if(nums != NULL) delete[] nums;
                if(fileOfNumbers != NULL) delete fileOfNumbers;

        }
       
private:
        bool isOK;
        unsigned int numCount;
        void ReadFile(char name[100]);
        void ErrHandling(errTypeE err);
        char fileName[100];
        ifstream *fileOfNumbers;
        int *nums;
       

};

Numbers::Numbers(char name[100])
{
        isOK = 1;
        nums = NULL;
        try{
                ReadFile(name);
        }catch(NumbersException *err){
                isOK = 0;
                err->PrintErrDescription();
        }
}

void Numbers::ReadFile(char name[100])
{
        strcpy_s(fileName, name);
        fileOfNumbers = new ifstream(fileName);
        if(!fileOfNumbers->good()) throw(new NumbersException(FILE_NOT_OPENED, fileName));

        int temp;
        numCount = 0;
        //This loop counts the items in the text file
        while(*fileOfNumbers >> temp){
                numCount++;
        }
        if(fileOfNumbers->bad()) throw (new NumbersException(ERR_READING_FILE, fileName));
        try {
                nums = new int[numCount+1];
        }
        catch (...) {
                throw (new NumbersException(MEM_ALLOC_FAILED));
        }

        numCount = 0;
        fileOfNumbers->clear();
        fileOfNumbers->seekg(0);
       
        //Now saving numbers in memory
        while(*fileOfNumbers >> nums[numCount]){
                numCount++;
        }
        if(fileOfNumbers->bad()) throw (new NumbersException(ERR_READING_FILE, fileName));
        fileOfNumbers->close();

}

void Numbers::PrintNums()
{
        for(unsigned int i = 0; i < numCount;i++) cout << nums[i] << endl;
}

int Numbers::Mod()
{
        int countN = 0;
        int mod = 0;
        int buff;
        for(int i = 0 ; i< numCount; i++)
        {
                buff = count(nums, nums+numCount, nums[i]);
                if(buff > countN){ countN = buff; mod = nums[i];}
        }
        return mod;


}

int Numbers::Ave()
{
        return(Sum()/numCount);
}

long long Numbers::Sum()
{
        return accumulate(nums,nums+numCount,0);
}

int Numbers::Max()
{
        return *max_element(nums,nums+numCount);
}

int Numbers::Min()
{
        return *min_element(nums,nums+numCount);
}

int Numbers::Mid()
{
        sort(nums, nums + numCount);
        return (nums[numCount/2]);
}


int _tmain(int argc, _TCHAR* argv[])
{
        Numbers nums("num.txt");
        if(nums.IsOK()) {
                nums.PrintNums();
                cout << "-------------------------------------------------------------------" << endl;
                cout << "Count: " << nums.Count() << endl;
                cout << "Sum: " << nums.Sum() << endl;
                cout << "Average: " << nums.Ave() << endl;
                cout << "Mid: " << nums.Mid() << endl;
                cout << "Mod: " << nums.Mod() << endl;
                cout << "Max: " << nums.Max() << endl;
                cout << "Min: " << nums.Min() << endl;
               
        }
        else
                cout << "There was an unrecoverable error. Sorry for inconvenience!" << endl;
       
        cout << "\nPress a key to exit..." << endl;
        _getch();
        return 0;
}

---

As far as I know, the long long data type is not (yet) part of C++, just C99. This thread agrees. http://www.thescripts.com/forum/thread63790.html

You can use long longs, as long as you are aware that they are non-standard.

Code:

---

int Numbers::Ave()
{
        return(Sum()/numCount);
}

---

Should this not maybe be returning a floating-point number, like a double? Otherwise the average of 1 and 2 and 2 is 1. At the very least consider rounding the result.

I'll make it double.

Mm'kay, but note that you'll have to make one of the numbers involved in the division a double as well, or the integral truncuation will still occur, having the result put into a double. So the average of 1, 2, and 2 would be 1.000000. :)

Something like this would work.

Code:

---

double Numbers::Ave()
{
        return((double)Sum()/numCount);
}

---

Also:

Code:

---

                if(nums != NULL) delete[] nums;
                if(fileOfNumbers != NULL) delete fileOfNumbers;

---

Deleting NULL is guaranteed to do nothing. So you could just go

Code:

---

                delete[] nums;
                delete fileOfNumbers;

---

It is bad practice to use such a local try catch (...) block around just a single alocation.
I recommend either getting rid of it, or placing it in main embodying the whole of it. For starters, this lets all your destructors get called etc and prevents the need to do the same thing all over the place. And since you clearly can't carry on past where the exception is caught anyway, the code shouldn't look like it might.

100 is not enough characters for a many file names. Use MAX_PATH (+1 for '\0') instead.

Quote:

---

It is bad practice to use such a local try catch (...) block around just a single alocation.

---

I want the class to handle its errors. Without try-catch, how this could be possible?

EDIT: Is there any difference between static_cast cast and C-style cast?

Quote:

---

Use MAX_PATH (+1 for '\0') instead.

---

As long as you are aware that MAX_PATH is Windows-only.

Many systems have unlimited pathname lengths, however, so the best way to handle this is with dynamic memory allocation.

PATH_MAX is a sort of Linux equivalent, but it isn't portable either. http://www.thescripts.com/forum/thread215365.html

Here's a somewhat entertaining take on MAX_PATH that I found while searching: http://blogs.msdn.com/tomholl/archiv...-max-path.aspx

Quote:

---

EDIT: Is there any difference between static_cast cast and C-style cast?

---

Yes and no. http://www.thescripts.com/forum/thread61547.html

For objects that can be casted with static_cast, there is no difference.

Also see this very nice post. http://cboard.cprogramming.com/showthread.php?p=621213

[edit]

Quote:

---

I want the class to handle its errors. Without try-catch, how this could be possible?

---

Make new return NULL on error instead of throwing an exception? [/edit]

Quote:

---

Make new return NULL on error instead of throwing an exception?

---

Oh, yes. Why not?

I defined MAX_PATH 260 at the beginning of the code as it is in <windef.h>.
Thanks for searchings dwks. I am reading them.

Quote:

---

I defined MAX_PATH 260 at the beginning of the code as it is in <windef.h>.

---

That's probably a bad idea, in case the value ever changes (not that it will!). I'd go (if you still want to use it):

Code:

---

#ifndef MAX_PATH
    #define MAX_PATH 260
#endif

---

After you include windef.h, of course.

So we should always use C++ casting.

[EDIT]

Code:

---

#ifndef MAX_PATH
    #define MAX_PATH 260
#endif

---

Including windef.h means losing portability. So what can be done?

Couple of notes:

Code:

---

//It doesn't use vector maybe for simplfication (?)

---

I can't see how managing the memory allocations yourself could be easier than letting the library do it. With a vector, for example, you wouldn't need to count the items in the file before loading them.

Code:

---

  ifstream *fileOfNumbers;

---

Why do you feel it is necessary to make this a member of the class? It is only used in one function.

Code:

---

int Numbers::Ave()
{
        return(Sum()/numCount);
}

---

This might still crash if the file was empty.

Quote:

---

So we should always use C++ casting.

---

In C++, yes. In C, of course, there's only one option. :)

This might crash as well.

Code:

---

int Numbers::Mid()
{
        sort(nums, nums + numCount);
        return (nums[numCount/2]);
}

---

Any code that assumes at least one number exists might crash.

Quote:

---

Any code that assumes at least one number exists might crash.

---

I will correct this problem using isOK.

Quote:

---

Why do you feel it is necessary to make this a member of the class? It is only used in one function.

---

For limiting its access.

Quote:

---

I can't see how managing the memory allocations yourself could be easier than letting the library do it.

---

I do this to practice memory allocation.

Quote:

---

Including windef.h means losing portability. So what can be done?

---

Aren't things such as "stdafx.h" and _tmain Microsoft compiler specific anyway?

Quote:

---

Quote:
Any code that assumes at least one number exists might crash.
I will correct this problem using isOK.

---

If the constructor fails, you'll have a completely unusable object - without a means to try loading another file. So you might just as well let the exception leave the constructor: user gets a valid object or nothing.
But I'm not that fond of using exceptions...

Quote:

---

Quote:
Why do you feel it is necessary to make this a member of the class? It is only used in one function.
For limiting its access.

---

How? If it is used locally in one function, its access would be even more limited? Your class is a wrapper around a numeric array, the ifstream is just a tool to get that array loaded from file.

Quote:

---

Quote:
I can't see how managing the memory allocations yourself could be easier than letting the library do it.
I do this to practice memory allocation.

---

That's a valid reason. But may-be update the comment then :)

Quote:

---

So you might just as well let the exception leave the constructor: user gets a valid object or nothing.

---

I don't know why I thought I can't throw exceptions from constructor (or it is a bad idea)!!! So it is possible, isn't it?

Quote:

---

How? If it is used locally in one function, its access would be even more limited? Your class is a wrapper around a numeric array, the ifstream is just a tool to get that array loaded from file.

---

Aha, now I understant you. I thought you say it should be global. You are right if the class design remains the same.