# My numeric analyzer (2)

• 07-26-2007
siavoshkc
My numeric analyzer (2)
I rewrote the code for it. Please tell me how it is:
Numbers.h:
Code:

```#include <fstream> #include <vector> #include <iostream> #include <numeric> #include <algorithm> #include <string> using namespace std; class Numbers{ public:         Numbers(string);        //Definitions are placed here to be inlined         __int64 Sum() {return accumulate(_b, _e, 0);}         double Ave() {return static_cast<double> (Sum()/numCount);}         int Mid() {return (nums[numCount/2]);}         void PrintNums() {for each (int val in nums) cout << val << endl;}         int Min(){return *min_element(_b, _e);}         int Max(){return *max_element(_b, _e);}         int Mod();         int Count()  {return numCount;}         private:         vector<int>::iterator _b,_e;         unsigned int numCount;         void ReadFile(string &fileName);         string fileName;         vector <int> nums; };```
Numbers.cpp:
Code:

``` #include "Numbers.h" Numbers::Numbers(string fileName) {         ReadFile(fileName);         //So we won't need to call these functions next times         numCount = static_cast <unsigned int> (nums.size()); _b = nums.begin(); _e = nums.end();                        sort(_b, _e);        //We need to sort to find the middle (Am I a poet?)        } void Numbers::ReadFile(string &fileName) {         cout << "Openning \""<< fileName <<"\"." << endl;          ifstream file(fileName.c_str(), ios::in);         if(file.is_open())         {                 int buf;                 cout << "Reading numbers." << endl;                 while(file >> buf) nums.push_back(buf);                                 if(!file.eof()) cerr << "WARNING: File was not read to end!" << endl;                 file.close();                }         else cerr << "ERROR: File could not be opened!" << endl; } int Numbers::Mod() {         unsigned int countN = 0;         int mod = 0;         unsigned int buff;         for each(int val in nums)         {                 buff = static_cast<unsigned int> (count(nums.begin(), nums.end(), val));                 if(buff > countN){ countN = buff; mod = val;}         }         return mod; }```
• 07-26-2007
anon
You seem to have too many casts. For example, std:.vector::size already returns an unsigned type. If you want to be more pedantic, you could make your size member a std::size_t.

Actually, why duplicate the vector size variable? You can always get the size from the vector (with zero overhead hopefully).

Code:

`double Ave() {return static_cast<double> (Sum()/numCount);}`
This cast seems to be out of place. If you don't want the average to be truncated, you'll need to cast either member of the division before dividing.
• 07-26-2007
siavoshkc
Quote:

This cast seems to be out of place. If you don't want the average to be truncated, you'll need to cast either member of the division before dividing.
Oh yes wrote it wrong in one of clean ups
• 07-26-2007
siavoshkc
OK, I changed them this way:
Code:

```#include <fstream> #include <vector> #include <iostream> #include <numeric> #include <algorithm> #include <string> using namespace std; class Numbers{ public:         Numbers(string);        //Definitions are placed here to be inlined         __int64 Sum() {return accumulate(_b, _e, 0);}         double Ave() {return (static_cast<double> (Sum()))/numCount;}         int Mid() {return (nums[numCount/2]);}         void PrintNums() {for each (int val in nums) cout << val << endl;}         int Min(){return *min_element(_b, _e);}         int Max(){return *max_element(_b, _e);}         int Mod();         size_t Count()  {return numCount;}         private:         vector<int>::iterator _b,_e;         size_t numCount;         void ReadFile(string &fileName);         string fileName;         vector <int> nums; };```
cpp:
Code:

```#include "stdafx.h" #include "Numbers.h" Numbers::Numbers(string fileName) {         ReadFile(fileName);         //So we won't need to call these functions next times         numCount = nums.size(); _b = nums.begin(); _e = nums.end();                        sort(_b, _e);        //We need to sort to find the middle (Am I a poet?)        } void Numbers::ReadFile(string &fileName) {         cout << "Openning \""<< fileName <<"\"." << endl;          ifstream file(fileName.c_str(), ios::in);         if(file.is_open())         {                 int buf;                 cout << "Reading numbers." << endl;                 while(file >> buf) nums.push_back(buf);                                 if(!file.eof()) cerr << "WARNING: File was not read to end!" << endl;                 file.close();                }         else cerr << "ERROR: File could not be opened!" << endl; } int Numbers::Mod() {         int mod = 0;         __w64 int buff, countN = 0;         for each(int val in nums)         {                 buff = count(nums.begin(), nums.end(), val);                 if(buff > countN){ countN = buff; mod = val;}         }         return mod; }```
And the runner:
Code:

```#include "Numbers.h" int main(int argc, char* argv[]) {         string fName;         if(argc>1) fName = argv[1];         else fName = "nume.txt";         Numbers nums(fName);         if(nums.Count()){                 nums.PrintNums();                 cout << "-------------------------------------------------------------------" << endl;                 cout << "Count: " << nums.Count() << endl;                 cout << "Sum: " << nums.Sum() << endl;                 cout << "Average: " << cout.precision(10) << nums.Ave() << endl;                 cout << "Mid: " << nums.Mid() << endl;                 cout << "Mod: " << nums.Mod() << endl;                 cout << "Max: " << nums.Max() << endl;                 cout << "Min: " << nums.Min() << endl;         }         cout << "\nPress a key to exit..." << endl;         _getch();         return 0; }```
• 07-26-2007
siavoshkc
• 07-26-2007
hk_mp5kpdw
Code:

```#include <fstream> #include <vector> #include <iostream> #include <numeric> #include <algorithm> #include <string> using namespace std; class Numbers{ public:         Numbers(string);        //Definitions are placed here to be inlined         __int64 Sum() {return accumulate(_b, _e, 0);}         double Ave() {return (static_cast<double> (Sum()))/numCount;}         int Mid() {return (nums[numCount/2]);}         void PrintNums() {for each (int val in nums) cout << val << endl;}         int Min(){return *min_element(_b, _e);}         int Max(){return *max_element(_b, _e);}         int Mod();         size_t Count()  {return numCount;}         private:         vector<int>::iterator _b,_e;         size_t numCount;         void ReadFile(string &fileName);         string fileName;         vector <int> nums; };```
You should avoid putting a "using namespace" statement anywhere within a header file as it opens up that entire namespace to whatever source file includes that header... intentional or otherwise. The namespace should be explicitly indicated where needed within the header. After that, any source file which includes a header can then put in a "using namespace" statement if necessary. For example:

Code:

```#include <fstream> #include <vector> #include <iostream> #include <numeric> #include <algorithm> #include <string> class Numbers{ public:         Numbers(std::string);        //Definitions are placed here to be inlined         __int64 Sum() {return std::accumulate(_b, _e, 0);}         double Ave() {return (static_cast<double> (Sum()))/numCount;}         int Mid() {return (nums[numCount/2]);}         void PrintNums() {for each (int val in nums) std::cout << val << std::endl;}         int Min(){return *std::min_element(_b, _e);}         int Max(){return *std::max_element(_b, _e);}         int Mod();         size_t Count()  {return numCount;}         private:         std::vector<int>::iterator _b,_e;         size_t numCount;         void ReadFile(std::string &fileName);         std::string fileName;         std::vector <int> nums; };```
And then in the source files:
Code:

```#include "Numbers.h" using namespace std; ...```
• 07-26-2007
anon
Code:

`void PrintNums() {for each (int val in nums) cout << val << endl;}`
I'm pretty sure this is not valid C++. Do you compile your code before posting it?
• 07-26-2007
siavoshkc
Quote:

I'm pretty sure this is not valid C++. Do you compile your code before posting it?
• 07-26-2007
laserlight
Ah, a language extension of MSVC. You might want to specify that next time, or simply dont use such an extension.
• 07-26-2007
siavoshkc
I didn't know that. Are you sure it is not standard? Maybe it is in standard proposal?
• 07-26-2007
laserlight
Quote:

I didn't know that. Are you sure it is not standard? Maybe it is in standard proposal?
Apparently it is part of C++/CLI. As such, it is not part of standard C++, but is a language extension. The next edition of the C++ Standard will probably include a for each loop, but with a slightly different syntax.
• 07-27-2007
siavoshkc
OK, I used this technique.
Code:

```int Numbers::Mod() {         int mod = 0;         __w64 int buff, countN = 0;         for(vector<int>::const_iterator i = nums.begin(); i < nums.end(); ++i)         {                 buff = count(nums.begin(), nums.end(), *i);                 if(buff > countN){ countN = buff; mod = *i;}         }         return mod; }```