2
\$\begingroup\$

20 different words are inputted using a string array.

A C++ program to display the alphabet which has occurred the maximum number of times as starting letter of a word in the list,and how many times it has occurred.

In this program i have also displayed a few other things too,like the longest length,shortest length,average length,maximum number of vowels,letter ending with s.

I would like to know how this program can be written more 'intelligently'.

#include<iostream.h>
#include<string.h>
int main()
{
int i,n,len=0,sum=0,j,max,min,c=0,c2=0,k=0;
int maxo=0,len1,maxi=0,c1=0,len2,counter=0,maxo1=0,len3;
 float avg;
char list[100][100] = { 0 };
char vow[] = "AEIOUaeiou";
char letter;
for(i=0;i<2;i++) 
{
cout<<"Enter word:  ";
gets(list[i]);

len=strlen(list[i]);                                
sum=sum+len;
cout<<"Length of word:  "<<len<<endl;
if(list[i][len-1]=='s')
{cout<<"The Word "<<list[i]<<" ends with s"<<endl;
 c2++;
 }

}
//Word input by user.Prints word along with length.       
min=strlen(list[0]);
max=strlen(list[0]);
//Initialising max and min.
for(i=0;i<2;i++)
 {
   if(strlen(list[i])<min)
   {min=strlen(list[i]);}
    if(strlen(list[i])>max)
    {max=strlen(list[i]);}

     }
  for(i=0;i<2;i++)
  {
  if(max==strlen(list[i]))
  cout<<"The max value of the lengths stored:"<<list[i]<<endl<<"Word count:"
  <<max<<endl;               
  if(min==strlen(list[i]))
  cout<<"The min value of the lengths stored:"<<list[i]<<endl<<"Word count:"
  <<min<<endl;
  }
//Max and Min value of string lengths are printed.
 avg=sum/2; 
 cout<<"Avg length:"<<avg<<endl;
 //Average value printed.
 cout<<"The number of words with s:"<<c2<<endl;
//Word ending with s.


 for (i = 0; i <2; i++) 

 {len1 = strlen(list[i]);
 for (k = 0; k < len1; k++) 
 {
    for (j = 0; j < strlen(vow); j++)
        //if (list[j][k] == vow[j])
        if (list[i][k] == vow[j])
            c++;
}
cout << "Number of vowels in line " << i << ": " << c << '\n';
if (c>maxo) maxo = c;
c = 0;
cout << "Maximum Vowel count so far:" << maxo << "\n\n";

 cout << "Maximum Vowel count:" << maxo << endl;
   }


 //Vowel maximum count 



 for(i = 0 ;i < 2 ;i++)
 { len3 = strlen(list[i]);
    letter = list[i][0];
 {for(j=0;j<len3;j++)
  if(list[i][j]==letter)
   counter++;

 }
 cout << "Number of identical letters as  first letter in line " << i << ": " << counter << '\n';
if (c>maxo1) maxo1 = counter;
counter = 0;
cout << "Maximum letter count so far:" << maxo1 << "\n\n";

 cout << "Maximum letter count:" << maxo1 << endl;
   }





fflush(stdin);
getchar();
return 0;
} 
\$\endgroup\$
  • 2
    \$\begingroup\$ I don't fully understand the title/the 2nd sentence of the question. It is a very long sentence and it is not clear to me what you mean by "alphabet". From the context it seems like you meant "character" instead. When you say occurred the maximum number of times, do you mean an actual upper bound, or rather most often (compared to the others)? \$\endgroup\$ – Raimund Krämer Feb 26 '18 at 12:58
  • \$\begingroup\$ No I am asking here to find the first letter of every string,find out how many time it occurs in that very string and print the maximum count value of the letter along with the string where it has occured the most. \$\endgroup\$ – Pole_Star Feb 26 '18 at 16:56
  • \$\begingroup\$ The statistical name for the most frequently occurring value is the modal value or simply mode. So you're interested in the mode of the first letters. \$\endgroup\$ – Toby Speight Feb 28 '18 at 8:34
4
\$\begingroup\$

Here are things I see:

Old Headers

#include<iostream.h>

iostream.h should be replaced with the modern day iostream. You can also consider replacing string.h with cstring. When you do this, you have to prefix all functions/classes from the standard library with std::. using namespace std; may seem shorter, but it is best avoided.


Indentation

int main()
{
int i,n,len=0,sum=0,j,max,min,c=0,c2=0,k=0;

You don't seem to have any sort of indentation set up in your main function. Pick some length, such as 4 spaces or a tab, and use it consistently. Indentation makes code easier to read.


Waaaaaaay too many variables in one place

int i,n,len=0,sum=0,j,max,min,c=0,c2=0,k=0;
int maxo=0,len1,maxi=0,c1=0,len2,counter=0,maxo1=0,len3;

This honestly looks horrible at first glance. Declaring variables at the beginning of a function is not something C++ requires. I suggest you move all of these variables close to where they are used, and use initialization instead of separate declaration and definition. For example:

len=strlen(list[i]);

Can be replaced with:

std::size_t len=strlen(list[i]);

Usage of C Style Multidimensional Arrays

char list[100][100] = { 0 };

C style multidimensional arrays are error prone, don't know their own size, and can easily decay to pointers. As such, they are best avoided. Consider using a std::arrays of std::strings, which are more versatile and less error prone.


Usage of C Style Strings

char vow[] = "AEIOUaeiou";

If you are using C++, then why are you using C style strings? Switch over to the much better std::string.


NEVER use gets

gets(list[i]);

This Stack Overflow answer tells you all you need to know. When you start using std::string, then use getline to read in a string.


Don't use endl unless you want to flush the output

... << endl;

Most people don't know when starting C++ that endl unnecessarily flushes the output. Switch over to \n.


Don't use strlen as a loop condition

for (j = 0; j < strlen(vow); j++)

Unless your compiler knows to optimize this out, strlen might be called every iteration in this code. To avoid this, put the length in its own variable.


Better Variable Names

Variable names such as i and c should only be reserved for loop indices. Use better variable names that clarify your intent.


fflush(stdin)

fflush(stdin)

https://stackoverflow.com/a/18170435/6525260. Don't play with undefined behavior.


getchar

In C++, a better way to wait for a character is:

std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
\$\endgroup\$
3
\$\begingroup\$

Besides the fundamental stuff pointed out in Arnav Borborah's answer, the code written here could greatly benefit from using functions to better organize the functionality of the code.

These are the specific things you want to do on the list of words:

  • most frequent starting letter
  • longest length
  • shortest length
  • average length
  • maximum number of vowels
  • number of words ending with 's'

Each of these can be turned into a function which takes in the list of words as a parameter, e.g. for the last one:

int CountWordsEndingWithS(const std::vector<std::string>& words)
{
  int count = 0;
  for (const auto& word : words)
  {
    if (!word.empty() && (word.back() == 's' || word.back() == 'S'))
    {
      ++count;
    }
  }
  return count;
}

Each function should have a clear purpose that is easily inferred from the function name, and the function code should be easily understood to fulfill that function's purpose.

You could then also put the reading of the words into its own function, leaving the main() function as clear and self-documenting:

int main()
{
  auto words = ReadWordsFromUser(20);
  std::cout << "Most frequent starting letter: " << FindMostFrequentStartingLetter(words) << '\n';
  std::cout << "Longest word length: " << FindLongestWordLength(words) << '\n';
  std::cout << "Shortest word length: " << FindShortestWordLength(words) << '\n';
  std::cout << "Average word length: " << CalculateAverageWordLength(words) << '\n';
  std::cout << "Number of vowels in word with the most vowels: " << FindLargestVowelCount(words) << '\n';
  std::cout << "Number of words ending with 's': " << CountWordsEndingWithS(words) << '\n';
}

(No need to put a return 0 at the end of main)

Edit: Adding an additional example for FindMostFrequentStartingLetter per request in the comments:

char FindMostFrequentStartingLetter(const std::vector<std::string>& words)
{
  auto letterCounts = std::array<int, 26>{};
  for (const auto& word : words)
  {
    if (!word.empty())
    {
      char startingLetter = word.front();
      if ('a' <= startingLetter && startingLetter <= 'z')
      {
        ++letterCounts[startingLetter - 'a'];
      }
      else if ('A' <= startingLetter && startingLetter <= 'Z')
      {
        ++letterCounts[startingLetter - 'A'];
      }
    }
  }
  return 'a' + std::distance(letterCounts.begin(), std::max_element(letterCounts.begin(), letterCounts.end()));
}

You'll need to tweak the code if you want it to return all letters that are tied for the most frequent, or if you also want to return the number of times it occurred.

\$\endgroup\$
  • \$\begingroup\$ In here can you write your code for the function of the most frquent starting letter. \$\endgroup\$ – Pole_Star Feb 26 '18 at 16:53
  • \$\begingroup\$ I would like to see how the code works with functions... \$\endgroup\$ – Pole_Star Feb 26 '18 at 17:01
  • \$\begingroup\$ No problem - FindMostFrequentStartingLetter added. This relies on max_element from the std library to do the actual work of finding which letter count is highest after we've calculated all the letter counts. \$\endgroup\$ – theosza Feb 27 '18 at 17:51
  • \$\begingroup\$ Be very wary of subtracting 'a' or 'A' from a character value in 'a'..'z' or 'A'..'Z' and expecting the result to be 25 or less - C++ doesn't make that guarantee, and there are real systems where that will cause UB. It's probably safest to cast to unsigned char and index into an array of size UCHAR_MAX + 1. Plus, you can then use std::isalpha() (and possibly std::tolower() and/or std::toupper()) to include the non-(US-)ASCII letters too. \$\endgroup\$ – Toby Speight Feb 28 '18 at 8:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.