Counting the amount of letters in a phrase!

[JoBe]

Hi guys,

Just wanted to ask you're opinions about this program I had to write for evening school, the idea is when you enter a phrase wich isn't longer then 80 letters, the programm will count the amount of each letter wich is written :!:

It doesn't have to keep account of 'a' or 'A', and I had to show each letter of the alphabet, just in case you we're wondering.

So, if you could just give me some pointers ;) in wich way I could improve it, I would appreciate very much :)

// AantalLetters.cpp : Johan Berntzen: 11/11/2004

#include <stdafx.h>
#include <iostream.h>
#include <iomanip.h>

int zoek(const char s[], char ch);

void main(void)
{
    int amount=0;
    char str[80], letter='a';

    cout<< " Write a phrase: ";
    cin.getline(str,80);

    for (int i=0;i<26;i++)
    {
        amount=zoek(str, letter);
        cout<< letter << " = "<< amount;
        cout<<setw(8);
        if(i%7==6)cout<<endl;
        letter++;
    }
    cin.get();
}

int zoek(const char s[], char ch)
{
    short a=0;

    for (int i=0;i<s[i];i++)
    {
        if (s[i]==ch)
            a=++a;
    }
    return a;
}

[Narue]

Before I make suggestions, I'll give you a cool version:

#include <algorithm>
#include <iostream>
#include <map>
#include <utility>
#include <string>

using namespace std;

map<char, int> get_frequency ( const string& s );
void show ( const pair<char, int>& freq_item );

int main()
{
  string s;

  cout<<"Enter a string: ";
  if ( getline ( cin, s ) ) {
    map<char, int> freq = get_frequency ( s );
    for_each ( freq.begin(), freq.end(), show );
  }
}

map<char, int> get_frequency ( const string& s )
{
  string::const_iterator begin = s.begin();
  string::const_iterator end = s.end();
  map<char, int> freq;

  for ( string::const_iterator it = begin; it != end; ++it )
    ++freq[*it];

  return freq;
}

void show ( const pair<char, int>& freq_item )
{
  cout<< freq_item.first <<": "<< freq_item.second <<endl;
}

On to the code!

>#include <stdafx.h>
This should be a avoided because it makes your code nonportable. To be more specific, you can only compile code using stdafx.h with Visual Studio AFAIK.

>#include <iostream.h>
>#include <iomanip.h>
These are old headers that technically aren't a part of C++. Remove the .h extension and you'll have the standard headers, but don'e forget to qualify standard names with std::.

>void main(void)
int main(). main never has, and probably never will return void. You can safely omit the void parameter because it means the same thing as an empty list in C++ and it's kind of ugly.

>letter++;
This relies too much on the character set. ASCII is not the only character set in existence, and some don't have the alphabet in sequential order. On such character sets your code will break.

The biggest problem with your code is that it's running time is quadratic. For each letter in the alphabet, you look over every letter in the string. For long strings this is a dreadful inefficiency and you can easily remove it by using a frequency table. My code uses the standard map class, which implements a frequency table through a binary search tree, but you can also use arrays with very little effort.

[JoBe]

Hi Narue,

Before I make suggestions, I'll give you a cool version:

Thanks for the example, but you know, I prefere to try and write my own ;)

On to the code!

>#include <stdafx.h>
This should be a avoided because it makes your code nonportable. To be more specific, you can only compile code using stdafx.h with Visual Studio AFAIK.

Thanks for the tip, I'll keep that in mind :!:

>#include <iostream.h>
>#include <iomanip.h>
These are old headers that technically aren't a part of C++. Remove the .h extension and you'll have the standard headers, but don'e forget to qualify standard names with std::.

Now that is strange, we're being teached this way of programming and you say that it's the old way, can you explain this a bit more :?:

>void main(void)
int main(). main never has, and probably never will return void. You can safely omit the void parameter because it means the same thing as an empty list in C++ and it's kind of ugly.

So, if I understand correctly, it's better to start writing main with 'int main()' can you explain why because I'm not asking a return from main :?:

>letter++;
This relies too much on the character set. ASCII is not the only character set in existence, and some don't have the alphabet in sequential order. On such character sets your code will break.

Well, we we're told to use ASCII, so :o I'll have to do it with ASCII, I understand that in certain circumstances it is better to use another way. But for this program, do you think it is good enough :?:

The biggest problem with your code is that it's running time is quadratic. For each letter in the alphabet, you look over every letter in the string. For long strings this is a dreadful inefficiency and you can easily remove it by using a frequency table. My code uses the standard map class, which implements a frequency table through a binary search tree, but you can also use arrays with very little effort.

Well, that's my biggest problem aswell, in my code I have to use an array of 26 integers wich represent 0 to 25 and each number stands for one letter, problem is, I don't know how to solve that problem :rolleyes:

Also, is there a way that I can use my code, but altered in a manner that it doesn't have to go trough the loop so many times :?:

To me it seems that now, the string allways has to go along with the letters each time it has to be checked :!: How could I change it in a manner that during the control of the letters the string stays put untill the total control of all numbers is done :?:

[jwenting]

Now that is strange, we're being teached this way of programming and you say that it's the old way, can you explain this a bit more

It's been that way since the introduction of C in the 1970s and only (relatively) recently been replaced (2000 or so) by the new system as default.
Many books still use the old system only, as do older compilers.
And newer compilers still accept the old way as an alternative to remain compatible with old code. Many teachers just haven't caught on yet (if they even know, many teachers create their course at some point and never bother to keep it (and their own knowledge) up to date after that.

So, if I understand correctly, it's better to start writing main with 'int main()' can you explain why because I'm not asking a return from main

Because the language standard asks for it would be a good enough reason. It's not strictly required I think (the compiler won't complain after all) but it's good practice.
Every program needs to have a return code, with 0 meaning (on most operating systems) there was no error.

Well, we we're told to use ASCII, so I'll have to do it with ASCII, I understand that in certain circumstances it is better to use another way. But for this program, do you think it is good enough

Always code for the future. In real projects you'd be told to code for ASCII and halfway through some analyst would storm into your office and tell you they have landed a new customer in Turkey or Malaysia who wants a localised version which means you'll have to be testing UTF-8. Better to be prepared for that from the outset, especially if it's an easy thing to do.

[JoBe]

@ jwenting, thanks for the explanation, I have no doubt that what I wrote would be unusable in other examples, but fact is, I have to use ASCII :o

It's our assignement :!:

@ Narue, am I correct when I say that your code is written in OO :?:

Because this: map<char, int> get_frequency
and this: string::const_iterator begin = s.begin(); type of code, we haven't seen in structured programming.

We are going to start Object Oriented in one or two weeks :!:

Oh yes, going to order following books by the way:

- 3D Math Primer for Graphics and Game Development.
ISBN: 1556229119
- Essential Mathematics for Computer Graphics
ISBN: 1852333804
- Concrete Mathematics: A Foundation for Computer Science
ISBN: 0201558025
- Art of Computer Programming Volumes 1-3 Box set
ISBN: 0201558025

What do you think :?:

[jwenting]

Do you have the bible?
The C++ Programming Language 3rd edition (Stroustrup)? That's pretty much required fodder for everyone who's into C++.
I don't know any of the books you mention, but the titles sound like fun :)

I've ordered "C ++ for Game Programmers" last week which should arrive in a few days, got good reviews at gamedev.net. Some reviewers on Amazon complained there's not enough DX and graphics specific code in there but IMO that's a good thing as there's tons of books about that. Instead it focusses on things like performance and other generic stuff with applications in (among other things) games.

If you want a good intro on 3D mathematics on a 2D screen I can recommend "Flights of Fantasy" by Christopher Lampton. It's certainly been out of print for over a decade though, it still deals with DOS and 640x480x256 resolution but the explanations of the math involved are good. ISBN 1878739182 Waite Group, 1993. Maybe a second-hand bookstore or library clearout sale can turn it up.
I've also learned quite a bit from another oldie, a book on writing graphics drivers for DOS in Assembly and C (and in fact I've once written a partial functionality driver for Turbo Pascal, sadly never completed as the project was cancelled)...

For the rest I'm afraid most of my recent (say last 5 years) library has been Java books as that's what I mainly use professionally.

[Narue]

>but you know, I prefere to try and write my own
That's why I showed you an example that couldn't possibly be accepted as homework. It's just a way for me to show off. ;)

>we're being teached this way of programming and you say that it's the old way, can you explain this a bit more
Before C++ was standardized, everyone basically agreed to use headers such as iostream.h as defined by The Annotated C++ Reference Manual written by Bjarne Stroustrup. When the language was standardized in 1998, these headers were edited heavily and the names were changed to remove the .h extension. You're being taught an old dialect of C++ that is quickly dying out.

>can you explain why because I'm not asking a return from main
If your compiler doesn't handle standard C++ then not returning a value from main results in undefined behavior. Undefined behavior is one of the worst things that your programs can have because it means they can do anything they like including (but not limited to) destructive operations on your computer such as wiping out data. Standard C++ defines main to return 0 (for success) if execution falls off the end, so you can omit a return statement safely.

However, just because you don't return a value explicitly doesn't mean that you can change the definition of main, which the standard states shall be (and when they use the word shall, you'd better do it or suffer undefined behavior):

int main()

or

int main ( int argc, char *argv[] )

or anything equivalent

Telling main to return void is undefined behavior, but in practice it's entirely possible that the program would simply fail to run if the run-time startup code sees a different call/return than it expected. Sadly, several systems support void main as an extension, but keep in mind that it's only an extension. Your code is not portable.

>Well, we we're told to use ASCII
You have to do what you have to do. Just remember that by assuming ASCII your code isn't as portable as it could be.

>I don't know how to solve that problem
ASCII letters are in sequential order. You can take advantage of that to create an array of 26 rather than an array of 128:

#include <cctype>
#include <iostream>

using namespace std;

int main()
{
  char letters[] = "abcdefghijklmnopqrstuvwxyz";
  int freq[sizeof letters - 1] = {0};
  char str[80];

  cout<<"Enter a string: ";
  if ( cin.getline ( str, sizeof str ) ) {
    for ( int i = 0; str[i] != '\0'; i++ ) {
      if ( isalpha ( str[i] ) )
        ++freq[tolower ( str[i] ) - 'a'];
    }

    for ( int i = 0; letters[i] != '\0'; i++ ) {
      if ( freq[i] != 0 )
        cout<< letters[i] <<": "<< freq[i] <<endl;
    }
  }
}

Otherwise you would need to account for every character in the character set, then discard what you don't want manually:

#include <cctype>
#include <iostream>

using namespace std;

int main()
{
  int freq[128] = {0};
  char str[80];

  cout<<"Enter a string: ";
  if ( cin.getline ( str, sizeof str ) ) {
    for ( int i = 0; str[i] != '\0'; i++ ) {
      if ( isalpha ( str[i] ) )
        ++freq[tolower ( str[i] )];
    }

    for ( int i = 0; i < 128; i++ ) {
      if ( isalpha ( static_cast<unsigned char> ( i ) ) && freq[i] != 0 )
        cout<< static_cast<unsigned char> ( i ) <<": "<< freq[i] <<endl;
    }
  }
}

Neither is better than the other simply because the former is more efficient for your purposes, but the latter is efficient if you need a more general frequency table for the character set.

>And newer compilers still accept the old way as an alternative to remain compatible with old code.
This is changing though. For example, Visual Studio .NET fails to compile prestandard C++.

>It's not strictly required I think (the compiler won't complain after all)
It is strictly required, and a compiler that doesn't complain is probably set to allow non-standard extensions and void main is one of them. But when set to disable extensions, all decent compilers should flag a warning.

>with 0 meaning (on most operating systems) there was no error
0 is success for all implementations, on all systems, or it's not C++.

>Narue, am I correct when I say that your code is written in OO?
Yes and no. I make use of standard objects, but the term generic programming (through templates) would be more appropriate than object oriented programming in this case.

>- Art of Computer Programming Volumes 1-3 Box set
Good choice. :) These are among my favorites. They're essential for any professional or hardcore hobbyist.

[JoBe]

>That's why I showed you an example that couldn't possibly be accepted as homework. It's just a way for me to show off. ;)

<<Oh, showboat LOL

>Before C++ was standardized, everyone basically agreed to use headers such as iostream.h as defined by The Annotated C++ Reference Manual written by Bjarne Stroustrup. When the language was standardized in 1998, these headers were edited heavily and the names were changed to remove the .h extension. You're being taught an old dialect of C++ that is quickly dying out.

<<Understood, I'll certainl mention this to him ;)

>...returning a value from main results in undefined behavior.

<<Understood.

>You have to do what you have to do. Just remember that by assuming ASCII your code isn't as portable as it could be.

<<Well, for this excercise I'll use it, but will keep in mind of the portability problem with it!

>Neither is better than the other simply because the former is more efficient for your purposes, but the latter is efficient if you need a more general frequency table for the character set.

<<Thanks for the examples, In recoding my own, I'll surely will try and see how and more important why you did it in that certain way :!:

>Yes and no. I make use of standard objects, but the term generic programming (through templates) would be more appropriate than object oriented programming in this case.

<<Well, I'll try that type of coding out within a few months(years) :lol:

>Good choice. :) These are among my favorites.

<< Well, you lead me to them ;)

@jwenting, nope haven't got Stroustrup, I saw that it's last print was from 2000, little bit old don't you think?

[Narue]

>little bit old don't you think?
IMO, the only better reference than The C++ Programming Language, is the C++ standard itself. Rest assured that everything in it is up to date enough for your needs.

[JoBe]

>little bit old don't you think?
IMO, the only better reference than The C++ Programming Language, is the C++ standard itself. Rest assured that everything in it is up to date enough for your needs.

Ok, I'll keep it in mind :!:

[JoBe]

Solution :?:

int main ()
{
    static int aantal[26];
    char str[80];

    cout<< "Geef een zin in: " <<endl;

    cin.getline(str,sizeof str);

    for (int i=0; str[i] !='\0'; i++)
    {
        for (char letters='a'; letters <='z';letters++)
        {
            if(str[i]==letters)
            aantal[letters -'a']++;
        }
    }

    for (i=0;i<26; i++)
    {
    cout<< char(i+'a') << " = " << aantal[i];
    cout<< setw(5);
    if(i%7==6)cout<<endl;
    }

    cin.get();

    return 0;
}

[Narue]

>Solution?
Well, it works if that's what you're asking. But the solution is still roughly quadratic in time complexity. The only real difference is that you've written the inner loop inline rather than inside of a function. The logic is basically the same.

You can remove the loop entirely, make the algorithm simpler, and double the performance by simply changing this:

for (int i=0; str[i] !='\0'; i++)
{
  for (char letters='a'; letters <='z';letters++)
  {
    if(str[i]==letters)
      aantal[letters -'a']++;
  }
}

To this:

for (int i=0; str[i] !='\0'; i++)
{
  if(str[i] >= 'a' && str[i] <= 'z')
    aantal[str[i] -'a']++;
}

[JoBe]

Well Narue,

It's not only doubling the performance, it's also much more comprehensible :!:

[JoBe]

Lattest version with function?

short zoek(const char str[], char letter);

int main ()
{
    static short aantal[26];
    char str[80], letter='a';

    cout<< "Geef een zin in: " <<endl;

    cin.getline(str,sizeof str);

    for (short i=0;i<26; i++)
    {
    aantal[letter -'a']= zoek(str, letter);
    cout<< char(i+'a') << " = " << aantal[letter-'a'];
    cout<< setw(5);
    if(i%7==6)cout<<endl;
    letter++;
    }

    cin.get();

    return 0;
}

short zoek(const char str[], char letter)
{
    static short aantal[26];

    for (short i=0; str[i] !='\0'; i++)
    {
        if(str[i]==letter)
        aantal[letter -'a']++;
    }
    return aantal[letter -'a'];
}

[Narue]

>Lattest version with function?
Why? You're reintroducing complexity and inefficiency. We've already established that nested loops aren't necessary to solve the problem, so you've just taken a step backward.

[JoBe]

I understand what you mean Narue, actually, it's my fault, the idea was and is in the program that we have to use a function or functions.

Also, there has to be an implementation of a string (cin.getline) and an array
(aantal[26])since these where the subjects that we saw before we got our assignment.
I understand that there are probably many other solutions for this, but we have to use those subjects so he can see wether we understood what we we're thaught.

So, next time I'll give a better explanation of what the exercise is ment to do :!:

[Narue]

#include <iostream>
#include <iomanip>

using namespace std;

void zoek(const char *str, int aantal[])
{
  for (int i=0; str[i] !='\0'; i++)
  {
    if(str[i] >= 'a' && str[i] <= 'z' )
      aantal[str[i] -'a']++;
  }
}

void show(const int aantal[], int size)
{
  for (int i=0;i<size; i++)
  {
    cout<< char(i+'a') << " = " << aantal[i];
    cout<< setw(5);
    if(i%7==6)cout<<endl;
  }
}

int main ()
{
  static int aantal[26];
  char str[80];

  cout<< "Geef een zin in: " <<endl;
  cin.getline(str,sizeof str);

  zoek(str, aantal);
  show(aantal, 26);

  cin.get();

  return 0;
}

1) Use at least one function: Check
2) Use a string and an array: Check
3) Simple and easy to follow: Check
4) Actually run in linear time: Check

Any questions?

[JoBe]

LOL, not really, that is really perfectly clear, I hope you won't mind me saying, but I'll use my solution to turn in tough, this way it was made 'mostly ;) ' by me.

Sure wish I could make it seem so easy like you did :cheesy:

I'm allready thrilled when I can write something like this:

#include <stdafx.h>
#include <iostream>
#include <iomanip>
using namespace std;

int main()
{	

	short s=0,y=0;
	short begin=0, eind=0;

	cout<< "Geef alle mogelijke combinatie's van een geheel getal 
                       met twee of meerdere opeenvolgende getallen" << endl;

	cout<< "Als het geheel getal gelijk is aan: ";
	cin>>s;cin.get();

	cout<< "Dan zijn er de volgende oplossingen: "<<endl;

	do
	{
		short z=0,w=0;
		y=++y;
		w=y;

			while(z<s)
			{
			z=z+w;			
			w++;
			}
	
		if(z==s && z!=y)
		{
			begin=y;
			eind=w;
			for (begin;begin<eind;begin++)
			{
				cout<<setw(3);
				cout<<begin;
			}
			cout<<endl;
		}
	}
	while (y<s);

	return 0;
}

And yeah, I'm showing of :lol:

Anyway, thanks for your help and time Narue :!: