Function only works once

[One Pea]

The 'multiArrayCipher' function calls on 2 other functions to fill a vector and then output that vector. However, only the first vector gets filled and outputted, the rest are just outputted as squares. For instance, if I input "Hello my name is" the output will be "ho e▢▢▢▢▢▢▢▢▢▢▢▢"

#include <iostream>
#include <cmath>
#include <string>
#include <fstream>
#include <vector>

using namespace std;

class functions
{
    private:
        char alphabet[26];
        string sentance;
        int counter=0;
        vector <char> multiArrayOne;
        vector <char> multiArrayTwo;
        vector <char> multiArrayThree;
        vector <char> multiArrayFour;

    public:
        void collectAlphabet ()
        {
            ifstream file;
            file.open("alphabet.txt");

            for (int j=0; j<26;j++)
            {
                file >> alphabet[j];
            }
        }

        void input ()
        {
            cout << endl;
            cout << "Write a sentence: ";
            getline(cin,sentance);

        }

        void arrayFill (int x, vector <char> & tempArray)
        {
            for (int j=x;j<=sentance.length();j+=4)
            {
                tempArray.push_back(1);
                tempArray[counter]=sentance[j];
                counter++;
            }

        }

        void arrayOutput (vector <char> tempArray)
        {
            for (int j=0;j<tempArray.size();j++)
            {
                cout << tempArray[j];
            }
        }

        void subCipher ()
        {
            cout << endl << "The new version is ";

            for (int i=0;i<sentance.length();i++)
            {
                if (sentance[i]==' ')
                    {
                        cout << " ";
                    }

                for (int j=0;j<26;j++)
                {
                    if (alphabet[j]==alphabet[23] && sentance[i]==alphabet[23])
                    {
                        cout << alphabet[0];
                    }

                    else if (alphabet[j]==alphabet[24] && sentance[i]==alphabet[24])
                    {
                        cout << alphabet[1];
                    }

                    else if (alphabet[j]==alphabet[25] && sentance[i]==alphabet[25])
                    {
                        cout << alphabet[2];
                    }

                    else if (alphabet[j]==sentance[i])
                    {
                        cout << alphabet[j+3];
                    }
                }
            }
            cout << endl << endl;
        }

        void multiArrayCipher()
        {
            arrayFill(0,multiArrayOne);
            arrayOutput(multiArrayOne);
            arrayFill(1,multiArrayTwo);
            arrayOutput(multiArrayTwo);
            arrayFill(2,multiArrayThree);
            arrayOutput(multiArrayThree);
            arrayFill(3,multiArrayFour);
            arrayOutput(multiArrayFour);
        }

};

int main()
{
    functions program;
    int input{};

    while (input!=5)
    {
        cout << "1. Substitution cipher" << endl;
        cout << "2. Multi-array cipher" << endl;
        cout << "3. Transposition cipher" << endl;
        cout << "4. Code breaker" << endl;
        cout << "5. Exit" << endl << endl;
        cout << "Enter the number of the option you want to proceed with: ";
        cin >> input;
        cin.ignore();

        switch (input)
        {
            case 1:
                program.collectAlphabet();
                program.input();
                program.subCipher();
                break;

            case 2:
                program.input();
                program.multiArrayCipher();
                break;


        }

    }


    return 0;
}

[icy1]

your alphabet array is probably filled with garbage if user immediately goes to option 2, because you didn't yet perform a population of alphabet.

Usually you want to do these things in the class constructor and not force the user to explicitly call collectAlphabet() method when really this is something that the class should do every time to initialize its data.

Also, does the alphabet.txt file ever change? If not, you may prefer this initializer list refactor:

// Consider naming your class with a capital letter, i.e. Functions.  It's more
//   stylistic, and also may avoid some trouble esp if you're using the std namespace, 
//   since many of those classes begin with lowercase letters.
class Functions  
{

// Simpler to align public: and private: with the opening of class bracket because, for the 
//   most part, it unnecessarily indents everything else.
private:

    // Adding an underscore is a common way to show private var at a glance
    vector<char> alphabet_;  

public:
    Functions()  // Constructor
    {
        alphabet_ = {'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'};
    }
};

If alphabet.txt does change, then you can either keep alphabet as char array as you currently have, or as a vector<char>, as long as you perform the insertions in constructor.

[Handy Andy]

Hello Shezshade,

I thought I had mentioned this before, but I could not find it. Maybe when I finish here.

Your function:

void arrayFill (int x, vector <char> & tempArray)
{
        for (int j=x;j<=sentance.length();j+=4)
        {
            tempArray.push_back(1);
            tempArray[counter]=sentance[j];
            counter++;
        }

}

is a problem. I had a run time error when "counter" was ==4, but the vector size was only 3. You can not put something into a vector element when it does not exit in the first place.

All you need for this function is:

void arrayFill (int x, vector <char> & tempArray)
{
        for (int j=x;j<=sentance.length();j+=4)
        {
            tempArray.push_back(sentance[j]);
        }

}

The other two lines if not working against you are not needed. When I tested it this worked fine.

Hopoe that helps,

Andy

[One Pea]

Thanks guys. I thought the push back function can only add space for a new element, not add the element itself?

[Handy Andy]

Hello Shezshade,

No it does not add space it adds what you tell it to. You could define a vector as:
std::vector<type> name(10); where the (10) will create a vector with 10 empty elements. Then you could use a sub script to access the vector;

Take a look at these two pages:
http://www.cplusplus.com/reference/vector/vector/emplace_back/
http://www.cplusplus.com/reference/vector/vector/push_back/

And if you have any questions let me know.

Hope that helps,

Andy

[icy1]

vector::reserve() reserves space (internally changes its 'capacity' variable value).
If you know exactly how much space you'll be needing, then you can preemptively call this method before any push_back s.

vector::push_back() appends element(s) if the capacity is fine. If capacity is not fine, it is automatically increased (I believe it's set to exactly what's needed).

Don't be afraid to dig around in the docs; Google is your friend =)