[C++] Vigenere cipher

[robdb]

I have to write an application which uses the Vigenere cipher with the Italian alphabet (only 21 characters, excluded J,K,W,X,Y). After a day of trying this is the code that I wrote

#include "vigenere.h"
#include <string>
using namespace std;

Vigenere::Vigenere(string str){
                       key = str;
                       key = StringToUpper(key);
                       for(int cnt = 0; cnt < key.length(); cnt++){
                               if(key[cnt] < 'A' || key[cnt] > 'Z')
                               key.erase(cnt,1);
                       }
                       
                       //nel caso in cui la chiave sia più corta del testo
                       for(int cnt = 0; cnt < text.length(); cnt++){
                               if(key.length() != text.length()){
                                               if(key.length() < text.length())
                                               key += key[cnt];
                                               }                
                       }
                                                                      
                       return;
}                   

//tavola di Vigenere con alfabeto italiano
char Vigenere::Table(string alphabet){
                       int temp;
                       temp = alphabet.length(); 
                       int ROWS(temp);
                       int COLS(temp);
                       char ALPH[temp];
                       for(int i = 0; i < temp; i++){
                               ALPH[i] = alphabet[i];
                       }
                       char VIG[ROWS][COLS];
                       
	                   for(int i = 0; i < ROWS; i++){		                       
		                           for(int j = 0; j < COLS; j++){
			                       VIG[i][j]=ALPH[(i + j)%21];
			                       }
                        }
                        return VIG[ROWS][COLS];
}

string Vigenere::StringToUpper(string convstr){
       for(int cnt = 0; cnt < convstr.length(); cnt++){
               convstr[cnt] = toupper(convstr[cnt]);
               }
       return convstr;
}

//elimina i caratteri speciali, tenendo solo le lettere
string Vigenere::EraseChar(string str){
       for (int i = 0; i < str.length(); i++){
           if(!(str[i] >= 'A' && str[i] <= 'Z'))
                       str.erase(i,1);
                       i--;
                       }
       return str;
}   

string Vigenere::encode(string text){
       string line;
              
       //cerca il valore corrispondente alla chiave nella prima colonna di VIG
       int index_k;
       for(int i = 0; i < ROWS; i++){ 
               if(key[i]==VIG[i][1])
               index_k = i;
       }
       
       //cerca il valore corrispondente al testo nella prima riga di VIG
       int index_t;
       for(int i = 0; i < COLS; i++){
               if(text[i]==VIG[1][i])
               index_t = i;
       }
       
       //calcola la lettera codificata
       for(int i = 0; i < text.length(); i++){
               line[i] = ALPH[(index_k + index_t)%21];
       }
       
       line = StringToUpper(line); 
       if(!line.empty()){
           line = EraseChar(line);
           }
           
       return line;
}

string Vigenere::decode(string text){
       string line;
              
       //cerca il valore corrispondente alla chiave nella prima colonna di VIG
       int index_key;
       for(int i = 0; i < ROWS; i++){
               if(key[i]==VIG[i][1])
               index_k = i;
       }
       
       //nella riga i-esima cerca la lettera corrispondente al carattere cifrato
       int index_d;
       for(int i = 0; i < COLS; i++){
               if(text[i] == VIG[index_key][i])
               index_d = i;
       }
       
       //calcola la lettera decodificata
       for(int i = 0; i < text.length(); i++){
               line[i] = ALPH[index_d];
       }
       
       line = StringToUpper(line);
       if(!line.empty()){
           line = EraseChar(line);
           }
           
       return line;
}

needless to say that it is wrong. please help me :(
PS comments are in italian, so if you don't understand something about the code, just ask.

[raptr_dflo]

Several problems:

You declare variables like ROWS, COLS, ALPH and VIG internally to your Table() method, so they are not accessible out side of it. Fix this by making them members of your Vigenere class, and then either:
+ pass 'alphabet' into your Vigenere::Vigenere() constructor and initialize them there, or
+ leave them uninitialized until you call your Table() method

Your EraseChar() method doesn't erase non-Italian letters out of the A-Z range. While it's probably slower, consider comparing each character in the input str against each character in ALPH.

Use the same EraseChar() method on key, in your constructor at line 8 above. The code in the constructor is almost the same as in EraseChar(), but is broken since you don't decrement i when you remove a character.

At line 14, it looks like you're trying to re-extend your key to be the same length as your text, but you don't have a text variable at that point, you're trying to append characters from the key that don't yet exist in it (when cnt is > key.length()) onto the end of it, and the test for lengths-not-equal is redundant (checking less-than is sufficient).

Let's get that much fixed and then see what problems remain.

[robdb]

I've modified the code and now it seems to be correct. But I can't visualize the encoded string as well as the decoded one in the output. here's the code

/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
//	vigenere.h
//
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////

#include <string>
using namespace std;

class Vigenere{
      private:
              string key;
              static char ALPH[21];
              static string StringToUpper(string);
              static string EraseChar(string);
      public:        
              Vigenere(string);
              string encode(string);
              string decode(string);
};


/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
//	vigenere.cpp
//
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////


#include "vigenere.h"
#include <string>
using namespace std;

Vigenere::Vigenere(string str){
       key = str;
       key = StringToUpper(key);
       if(!key.empty()){
           key = EraseChar(key);
       }                            
          
       return;
}

char Vigenere::ALPH[21] = {'A','B','C','D','E','F','G','H','I','L','M','N','O','P','Q','R','S','T','U','V','Z'};

//converts every letter to uppercase in the parameter convstr
string Vigenere::StringToUpper(string convstr){
       for(int i = 0; i < convstr.length(); i++){
               convstr[i] = toupper(convstr[i]);
               }
       return convstr;
}

//deletes the special characters, keeps only letters of the Italian alphabet
string Vigenere::EraseChar(string str){
       for (int i = 0; i < str.length(); i++){
           if(!(str[i] >= 'A' && str[i] <= 'Z'))
                       str.erase(i,1);
                       //i--;
       }
       
       return str;
}   

string Vigenere::encode(string text){
       text = StringToUpper(text); 
       if(!text.empty()){
           text = EraseChar(text);
       }       
                     
       int index_k[key.length()];  		//key's indexs array           
       for(int i = 0; i < key.length(); i++){
               for(int j = 0; j < 21; j++){          
                       if(key[i] == ALPH[j]);
                       index_k[i] = j;
               }        
       }
       
       int index_t[text.length()];  		//text's indexs array
       for(int i = 0; i < text.length(); i++){
               for(int j = 0; j < 21; j++){
                       if(text[i] == ALPH[j])
                       index_t[i] = j;
               }        
       }
             
       string line;
       //coded letter
       for(int i = 0; i < text.length(); i++){
               line[i] = ALPH[(index_k[i%key.length()] + index_t[i])%21];
       }
       
       return line;
}

string Vigenere::decode(string text){
       text = StringToUpper(text); 
       if(!text.empty()){
           text = EraseChar(text);
       }
       
       int index_k[key.length()];  		//key's indexs array
       for(int i = 0; i < key.length(); i++){
               for(int j = 0; j < 21; j++){
                       if(key[i] == ALPH[j])
                       index_k[i] = j;
               }
       }
       
       int index_c[text.length()];  		//ciphertext's index array
       for(int i = 0; i < text.length(); i++){
               for(int j = 0; j < 21; j++){
                       if(text[i] == ALPH[j])
                       index_c[i] = j;
               }
       }
       
       string line;
       //decoded letter
       for(int i = 0; i < text.length(); i++){
               line[i] = ALPH[(index_c[i] - index_k[i%key.length()] + 21)%21];
       }
        
       return line;
}


/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
//
//	main.cpp
//
/////////////////////////////////////////////////////////////////////////////////////////////////////////////////


#include <cstdlib>
#include <iostream>
#include <string>
#include "vigenere.h"
using namespace std;

int main()
{
    string chiave;
    cout << "Insert key:\n";
    cin >> chiave;
    Vigenere cipher(chiave);
    
    string original;
    cout << "Insert plaintext:\n";
    cin >> original;
    
    string encrypted = cipher.encode(original);
    string decrypted = cipher.decode(encrypted);
    
    cout << "Encrypted: " << encrypted << endl;
    cout << "Decrypted: " << decrypted << endl;    
               
}

how can I fix that?

[raptr_dflo]

You have a spurious semicolon at the end of line 74.

Your EraseChar() method is still incorrect. See what happens if you have two or more successive characters not in the range 'A'-'Z'.

In encode() and decode(), what is the value of k_index if key isn't one of the characters in ALPH? Same for t_index and text? If key and text are only supposed to contain characters from ALPH, then you still need to rewrite EraseChar() to remove the non-ALPH characters.

Try adding cout lines after you've called StringToUpper() on key, and again after you've called EraseChar(). Make sure your input key has a mix of lower and upper-case letters, both within ALPH and not, along with some numbers and punctuation symbols, to make sure those two functions are working as intended.

[robdb]

I've modified the erasechar function as you suggested. Now it deletes all characters not included in ALPH.

string Vigenere::EraseChar(string str){
        for(int i = 0; i < str.length(); i++){
              for(int j = 0; j < 21; j++){
                    if(str[i] != ALPH[j])
                    str.erase(i,1);
              }
        }
        return string;
}

but i can't visualize the output yet. Is there something wrong in main.cpp?

[raptr_dflo]

Read the code you just posted, carefully. First, what are you returning? Once you fix that, if you can't see what's wrong with the code, try inserting cout << "str = '" << str << "'" << endl; before line 2, and again after line 7.

There's nothing like debug-prints for tracking down problems!

[robdb]

I've tried to modify the function following your advices. This is the result

string EraseChar(string str){
       char ALPH[21] = {'A','B','C','D','E','F','G','H','I','L','M','N','O','P','Q','R','S','T','U','V','Z'};
       
       str = StringToUpper(str);
       cout << "str = '" << str << "'" << endl;
       
       int match;
              
       for(int i = 0; i < str.length(); i++){
               for(int j = 0; j < 21; j++){
                       if(str[i] == ALPH[j]){
                       continue;  
                       match = 1;
                       }
                       else
                       match = 0;              
               }
               
               if(match == 1) continue;
               else if(match == 0)
               str.erase(i,1);               
       }
       cout << "str = '" << str << "'" << endl;
       return str;
    }

but now the function keeps only even letters. For example, the key is ROBERTO, the output is OET. the problem is...I can't find out a solution. :(

[raptr_dflo]

I haven't tested it, so apologies if I have any syntax errors in it, but try this:

string Vigenere::EraseChar(string str){
    for(int i = 0; i < str.length(); i++){
        bool inAlph = false;
        for(int j = 0; j < 21; j++){
            if(str[i] == ALPH[j]){
                inAlph = true;
                break;  // don't need to keep checking against the rest of ALPH
            }
        }
        if (!inAlph) {
            str.erase(i,1);
            i--;
        }
    }
    return str;
}

[robdb]

thank you very much, now it works. I think I only have one last problem :( with functions encode and decode.
when I write in main

string encrypted = cipher.encode(original);
string decrypted = cipher.decode(encrypted);
 
cout << "Encrypted: " << encrypted << endl;
cout << "Decrypted: " << decrypted << endl;

nothing appears in the output

[robdb]

I'm trying to fix problems using debug-prints as you suggested me. So I've modified the function encode in this way

string encode(string text){
       char ALPH[21] = {'A','B','C','D','E','F','G','H','I','L','M','N','O','P','Q','R','S','T','U','V','Z'};
       text = StringToUpper(text); 
       if(!text.empty()){
           text = EraseChar(text);
       }
       
       cout << "String text: " << text << endl;
              
       int index_k[key.length()];  //key's indexs array           
       for(int i = 0; i < key.length(); i++){
               for(int j = 0; j < 21; j++){
                       if(key[i] == ALPH[j])
                       index_k[i] = j;
               }
       cout << "Index key " << i << ": " << index_k[i] << endl;                
       }
       
       int index_t[text.length()];  //text's indexs array
       for(int i = 0; i < text.length(); i++){
               for(int j = 0; j < 21; j++){
                       if(text[i] == ALPH[j])
                       index_t[i] = j;
               }
       cout << "Index text " << i << ": " << index_t[i] << endl;                
       }
             
       string line;
       //coded letter
       for(int i = 0; i < text.length(); i++){
               line[i] = ALPH[(index_k[i%key.length()] + index_t[i])%21];
               cout << "Coded Letter " << i << ": " << line[i] << endl;
       }
       
       return line;       
    }

and everything works, all coded letters are printed. So, what could be the problem? Why can't I visualize the output when I call the function in main?
:(

[raptr_dflo]

Right at line 1, you've omitted the class-name, unless that was a copy-and-paste error:

string Vigenere::encode(string text){

Could that be the problem? Otherwise, your code looks fine. Try one more cout right before your return from encode():

cout << "final encrypted text: " << line << endl;