CODE CHECK.!!

[samoukos]

HELLO .... I WROTE THIS CODE .... IT'S PORPOSE IS TO READ A GREEK TEXT
USING ASCII CODING AND THEN COUNTING HOW MANY TIMES EACH WORD
APPEARS...THEN IT MUST PRINT IT OUT WITH ASCEDING SEQUENCE.. CAN YOU
PLEASE CHECK THIS CODE AND TELL ME IF THERE ARE ANY MODIFICATIONS THAT
CAN BE MADE IN ORDER TO MAKE IT FASTER AND BETTER...THANK YOU....

HERE IS THE CODE::

#include<iostream>
#include<cstdlib>
#include<fstream>
using std::ifstream;
using std:fstream;
using std::endl;
using std::ios;

int main ()
{
ifstream fin;
ofstream fout;
int i,j,q,NumberUsed,max,imax;
char alpha[1000000];
int sum[255];
fin.open("TEXT.txt");
fout.open("TEXT.out");
//-----------------------------------
for (i=0;i<1000000;i++)
{
alpha=0;
}
//----------------------------------
for (i=0;i<255;i++)
{
sum=0;
}
//----------------------------------
//----------------------------------
i=0;
while (!fin.eof())
{
alpha=fin.get();
i++;
}
NumberUsed=i;
//----------------------------------
for (i=0;i<NumberUsed;i++)
{
q=alpha;
//cout << q << endl;
sum[q]=sum[q]+1;
//cout << sum[q]<< endl;
}

//----------------------------------
// Displays ascii values as entered
// used for tracing
//for (i=0;i<NumberUsed;i++)
//{
//fout << alpha+1-1 << " ";
//}
//----------------------------------
//
// Outputs character frequency
//for (i=0;i<255;i++)
//{
//fout << i <<" "<< sum<<endl;
//}
//----------------------------------
max=0;
for (j=0;j<26;j++)
{
for (i=97;i<122;i++)
{

if (sum>max)
{
max=sum;
imax=i;
}
}

if (sum[32]>max)
{
max=sum[32];
imax=32;
}
fout << char(imax)<< " "<<sum[imax]<<endl;
sum[imax]=0;
max=0;
}
//----------------------------------

fin.close();
fout.close();
fout << endl;
return 0;
}

 

[Matthias Buelow]

CAN YOU PLEASE CHECK THIS CODE AND TELL ME IF THERE ARE ANY MODIFICATIONS THAT
CAN BE MADE IN ORDER TO MAKE IT FASTER AND BETTER...THANK YOU....

YES THERE ARE. YOU'RE WELCOME.

 

[Alf P. Steinbach]

* samoukos:

HELLO .... I WROTE THIS CODE .... IT'S PORPOSE IS TO READ A GREEK TEXT
USING ASCII CODING AND THEN COUNTING HOW MANY TIMES EACH WORD
APPEARS...THEN IT MUST PRINT IT OUT WITH ASCEDING SEQUENCE.. CAN YOU
PLEASE CHECK THIS CODE AND TELL ME IF THERE ARE ANY MODIFICATIONS THAT
CAN BE MADE IN ORDER TO MAKE IT FASTER AND BETTER...THANK YOU....

Please don't shout.

Note that ASCII does not define any greek characters: presumably what
you mean is using an encoding with one byte per character.

Also, it doesn't seem like the program counts words, but character
instances (e.g. number of occurences of 'A' in the text).

HERE IS THE CODE::

#include<iostream>
#include<cstdlib>
#include<fstream>
using std::ifstream;
using std:fstream;
using std::endl;
using std::ios;

int main ()
{
ifstream fin;

Indentation is a good idea. E.g. add four spaces at the start of lines
inside { and }.

ofstream fout;
int i,j,q,NumberUsed,max,imax;

These variables should most probably be declared locally where they're
used. It's a good idea to use consistent naming convention. I.e.
numberUsed or number_used, not NumberUsed which is unlike the others.

char alpha[1000000];

From the code below this is used to store the text.

A std::string is much more safe and convenient.

int sum[255];

If sum is the number of occurences of the character encoded as value
i, then the above array can't count occurences of a character encoded as
255, because valid indices range from 0 to 254, inclusive.

The assumption of no 255-characters may be valid, or not.

fin.open("TEXT.txt");
fout.open("TEXT.out");
//-----------------------------------
for (i=0;i<1000000;i++)
{
alpha=0;
}

Level 1 correction: use symbolic names for "magic" numbers such as
1000000, and use descriptive names such as "text" rather than "alpha".

Level 2 correction: use a locally declared loop variable,

for( int i = 0; i < textBufferSize; ++i )
{
text = 0;
}

Level 3 correction: do this initialization in the declaration,

size_t const textBufferSize = 1000000;

char text[textBufferSize] = {0};

Level 4 correction: use a std::string instead,

std::string text;

//----------------------------------
for (i=0;i<255;i++)
{
sum=0;
}

Level 1, 2 and 3 corrections as noted for 'alpha' above.


Rest snipped, I think the above comments enough for now.

Cheers & hth.,

- Alf