# Thread: My numeric analyzer (2)

1. ## 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;
string fileName;
vector <int> nums;

};```
Numbers.cpp:
Code:
```#include "Numbers.h"

Numbers::Numbers(string 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?)
}

{
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;
}```

2. 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.

3. 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

4. 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;
string fileName;
vector <int> nums;

};```
cpp:
Code:
```#include "stdafx.h"
#include "Numbers.h"

Numbers::Numbers(string 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?)
}

{
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;
}```

6. 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;
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;
std::string fileName;
std::vector <int> nums;
};```
And then in the source files:
Code:
```#include "Numbers.h"
using namespace std;

...```

7. 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?

8. I'm pretty sure this is not valid C++. Do you compile your code before posting it?

9. Ah, a language extension of MSVC. You might want to specify that next time, or simply dont use such an extension.

10. I didn't know that. Are you sure it is not standard? Maybe it is in standard proposal?

11. 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.

12. 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;
}```