0

Hello I am trying to generate a random array of the length that the user inputs. My array should then print and display the occurences of those letters in the array. So far this only prints up to the letter g and the occurences are incorrect. If someone could tell me what I am doing wrong it would help alot. Thank you.

#include <iostream>
#include <cstring>
#include <ctime>
#include <cstdlib>

using namespace std;


int main()
{
    srand(time(0));
    int i, num;
    char ch;
    char chars[]={'a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z'};
    int freq[26]={0};
    cout << "How many letters do you want in your string? ";
    cin >> num;

    for (i=0; i < num; i++)
    {
        ch = chars[rand()%26];
        chars[i]=ch;
        freq[i] +=1;
        cout << ch;
    }

    for (char lower = 'a'; lower <='z'; lower++)
    {
        cout << "\nLetter" << lower << "is " << freq[lower] << "times";
    }
}
Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141
starlight96
  • 3
  • 3
  • 1
    Read up on the [modern random facilities](https://en.cppreference.com/w/cpp/numeric/random) provided by modern C++. *Don't* use `srand`/`rand` in new code, *please*. You are also introducing bias through your use of modulo, a `std::uniform_int_distribution` would be much better. – Jesper Juhl Nov 08 '19 at 19:16
  • How are those random arrays supposed to be generated? Should the characters of the array itself be generated independently of each other? If yes, then I don't think using `rand` is a good idea. – Empty Space Nov 08 '19 at 19:19

4 Answers4

2

Problem 1

The lines

chars[i]=ch;
freq[i] +=1;

are not right. You need to use:

int index = ch - 'a';
freq[index] += 1;

Problem 2

The index in the for loop for printing the data is not correct either.

You need to use:

for (char lower = 'a'; lower <='z'; lower++)
{
    int index = lower - 'a';
    cout << "\nLetter" << lower << "is " << freq[index] << "times";
}

Important Note

It is worth noting that the C++ standard does not guarantee that lower case letters are contiguous. (Thanks @MartinBonner). For instance, if your system uses EBCDIC encoding your program won't work.

To make your code robust, it will be better to use a std::map.

int main()
{
    srand(time(0));
    int i, num;
    char ch;
    char chars[]={'a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z'};

    std::map<char, int> freq;

    // Initialize freq.
    for ( ch : chars )
    {
       freq[ch] = 0;
    }

    cout << "How many letters do you want in your string? ";
    cin >> num;

    for (i=0; i < num; i++)
    {
        ch = chars[rand()%26];
        freq[ch] +=1;
    }

    for (auto item : freq )
    {
        cout << "\nLetter" << item.first << "is " << item.second << "times";
    }
}
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • 1
    It is worth noting that the C++ standard does not guarantee that lower case letters are contigious (I don't even think it guarantees they are in order). This is not *purely* of academic interest: there *are* systems where it is not true (those that use EBCDIC). – Martin Bonner supports Monica Nov 08 '19 at 19:52
1

You might wanna give a look to C++11 Pseudo-random number generation here is a short way of generating the range that you want using this:

#include <algorithm>
#include <array>
#include <random>
#include <vector>

using namespace std;

int main()
{
    int arraySize = 35;

    mt19937 engine{random_device{}()};

    uniform_int_distribution<> dist{'a', 'z'};

    vector<char> vec;

    generate_n(back_inserter(vec), arraySize, [&]() { return static_cast<char>(dist(engine); }));

    //To count occurrences 

    array<int, 26> freq;

    for (auto c : vec) { ++freq[c-'a']; }

    return 0;
}
Gerard097
  • 815
  • 4
  • 12
0

You should not write into chars, and freq should be extended to cover the a...z range (the ASCII codes), which it does not. Also, increase at index ch, not at i.

I do not even know that range from the top of my head, but it could be modified to track all possible bytes instead (0...255), see result on https://ideone.com/xPGls7
List of changes:

int freq[256]={0}; // instead of int freq[26]={0};
// chars[i]=ch; is removed
freq[ch] +=1; // instead of freq[i] +=1;

Then it works.

tevemadar
  • 12,389
  • 3
  • 21
  • 49
  • Thank you. this worked perfectly. I really appreciate it – starlight96 Nov 09 '19 at 02:00
  • @starlight96 May i ask why you are not accepting this answer if this helped you? It seems like youre new to StackOverflow, i recommend you to take a look at https://stackoverflow.com/help/someone-answers – Eric Nov 12 '19 at 12:50
  • Thank you. I did not know I needed to accept an answer. Will do now. – starlight96 Nov 13 '19 at 13:02
0

Using lambda functions to do most of the work.

#include <algorithm>
#include <functional>
#include <iostream>
#include <map>
#include <numeric>
#include <ostream>
#include <random>
#include <string>
#include <utility>
#include <vector>

using namespace std::string_literals;

int main()
{
    std::mt19937::result_type seed = std::random_device{}();
    auto engine = std::mt19937(seed);
    auto dist = std::uniform_int_distribution<>('a', 'z');
    auto random_letter = [&engine, &dist]() { return static_cast<char>(dist(engine)); };

    std::cout << "How many letters do you want to generate? "s;
    int n;
    if (!(std::cin >> n)) { return EXIT_FAILURE; }

    auto letters = std::vector<char>();
    std::generate_n(std::back_inserter(letters), n, random_letter);

    auto zero = std::map<char, int>();
    auto const frequencies = std::accumulate(std::cbegin(letters), std::cend(letters), zero, 
        [](auto& acc, auto c)
        { 
            ++acc[c]; 
            return acc;
        });

    for (auto const [c, freq] : frequencies)
    {
        std::cout << "The letter '"s << c << "' appeared "s << freq << " times." << std::endl;
    }

    return 0;
}
user515430
  • 298
  • 1
  • 3
  • 7