Encryption Program Problem

[mikesnuggets22]

Having trouble with this code that resembles a Caesar cipher (alphabet letter shift).

The task is to: (All lowercase letters)

1. Design a function, encrypt, that when applied to an unencrypted string un_str, transforms un_str to an encrypted string en_str using a right shift of length s to the letters of the alphabet with wraparound.
2. Design a function decrypt that when applied to an encrypted string en_str, transforms en_str to a decrypted string. The string en_str will have been encrypted with a cipher of length s.

Input: Shift length s, unencrypted string un_str, encrypted en_str
Output: String un_str and its encryption using function encrypt, String en_str and its decryption using function decrypt

Finished Program should be able to:

- Prompt for and get shift length x
- Prompt for and get unencrypted string un_str
- Encrypt un_str into en_str
- Prompt for and get encrypted string en_str
- Decrypt en_str into un_str


This is what I have so far, it's obviously amateur's work and I know there are SEVERAL errors as it is (though I'm not sure where ~). I'm trying to have it so that the program asks for a shift length, followed by offering a decrypt or encrypt option, followed by prompting for an entered string, and have the ability to perform those processes. Unfortunately I'm stuck. The string part, I really need help on. Thanks in advance

Title: 9.cpp


#include "9f.h"
#include <iostream>
#include <stdio.h>
#include <string>
using namespace std;

int main(void)
{
	char un_str[100];
	int shift;
	int crypt;

	std::cout << "Enter a desired shift length -> "; 
	std::cin >> shift;

	std::cout << "Press 1 to encrypt or 0 to decrypt: ";
	std::cin >> crypt;
	while (getchar() != '\n');
	if (crypt == 1)
		encrypt(shift);
	else 
	{
		decrypt(shift);
	}
		
	std::cout << "Enter a string -> ";
	std::cin.getline(un_str, 100);

	return 0;
}

Title: 9f.cpp


#include "9f.h"
#include <iostream>

void encrypt(int shift) // prototypes of functions used in the code
{
	char en_str;
	std::cout <<
	en_str = getchar();
	while(en_str != '\n')
	{
		if (ch == ' ')
			putchar(en_str);
		else
		{
			if(shift == 1)
				putchar (en_str + shift);
			else
				putchar (en_str - shift);
		}
		en_str = getchar();
	}
	putchar(en_str);

	return 0;
}

void decrypt(int shift)
{
	shift = -1 * shift;
	encrypt(shift);
	return 0;
}

Title: 9f.h


void encrypt (int shift);
void decrypt (int shift);

[monarch_dodra]

You are on the right track. One of your problems is the successive calls of getchar. Remember that not only does it read the last letter of your input, it also REMOVES that letter from your input.

when you write:

while (getchar() != '\n');

if the character is not \n, then it is lost forever! you are right to do the test, but you should also try not to loose that character.

If I were you, I would modify your encrypt and decrypt functions to not require user input. Keep all the user input management inside your main. I my opinion, this is what your encrypt should look like:

char encrypt(char input, int shift) //Takes a char, a shift, and returns the encrypted char
{
    char encrypted;
    encrypted= input + shift;
    if (encrypted > 122) 
    {
        encrypted -= 26;
    }
    return encrypted;
}

The above code assumes 0<shift<26. See http://en.wikipedia.org/wiki/ASCII#A...ble_characters for an explanation of if encrypted > 122. There is a general formula, but more complicated, for shifts > 26.

I'm also confused about your main. You call the encrypt/decrypt functions before asking the user for a string. What is up with that? Try something like this:

int main(void)
{
	char un_str[100];
	char en_str[100];
	int shift;
	int crypt;
	char current_letter;

	std::cout << "Enter a desired shift length -> "; 
	std::cin >> shift;

	std::cout << "Press 1 to encrypt or 0 to decrypt: ";
	std::cin >> crypt;
		
	std::cout << "Enter a string -> ";
	std::cin.getline(un_str, 100);

	//Loop begin
	//Validate input, ie not space etc
	en_str[i] = encrypt(current_letter, shift);
	//loop end

	cout << en_str;

	return 0;
}

You are on the right track. Keep going.

If you want my personal advice, don't worry about bad user input in the beginning, as it is distracting you from thinking about the real problem. Solve the real problem, then worry about correcting user input. By the same logic, concentrate on encoding only right now. You'll worry about the if logic later.

[mikesnuggets22]

Is there a way to do it so that a 122 and +- 26 isn't necessary? As far as that main function, yeah I'm confused, not sure where to go with the strings

[monarch_dodra]

Originally Posted by mikesnuggets22

Is there a way to do it so that a 122 and +- 26 isn't necessary?

Yes, but they are much heavier code wise, and you'd have to test each letter one by one (if 'a' return 'b' etc). The idea is that a lowercase letter has a binary value between 97 and 122, included. a is 97 and z is 122. The problem is that if you try to shift the letter z, by doing z + shift, you'll have a number above 122.

If you want to avoid the if, use this formula, it will always work.

encrypted = ((letter - 97 + shift) % 26) + 97;

Another solution would be doing giant switch statement, or using while loops or other. But I don't think they would be as effective.

Here is a solution, but I wouldn't use it.

shift = shift%26;
encrypt = non_encrypt;
for (int i=0; i<shift; i++)
{
    switch encrypt
    {
        case 'a': encrypt = 'b'; break;
        case 'b': encrypt = 'c'; break;
        case 'c': encrypt = 'd'; break;
        ...
        case 'z': encrypt = 'a'; break;
    }
}

This will shift your letter by 1, and you run it shift times.

[laserlight]

Originally Posted by monarch_dodra

The idea is that a lowercase letter has a binary value between 97 and 122, included. a is 97 and z is 122.

In theory, though no longer in practice as far as I know, those characters need not have those values, and need not even be contiguous in alphabetical order.

Originally Posted by monarch_dodra

If you want to avoid the if, use this formula, it will always work.

encrypted = ((letter - 97 + shift) % 26) + 97;

It would be somewhat more readable to write:

encrypted = ((letter - 'a' + shift) % 26) + 'a';

plus it also has the theoretical benefit of working on any character set for which the lowercase letters of the English alphabet are in order and contiguous.

Originally Posted by monarch_dodra

Another solution would be doing giant switch statement, or using while loops or other. But I don't think they would be as effective.

A switch would guarantee that it works regardless of how those characters are laid out in the character set, but then this advantage is quite theoretical.

[monarch_dodra]

Originally Posted by laserlight

In theory, though no longer in practice as far as I know, those characters need not have those values, and need not even be contiguous in alphabetical order.

While I understand character mapping is environment dependent, I was pretty sure all modern OS followed the ASCII scheme for at least the first 127 symbols, and the rest locale dependent. What do you mean: "no longer in practice"? Do you just mean that there is no guarantee they follow this scheme?

Originally Posted by laserlight

It would be somewhat more readable to write:

encrypted = ((letter - 'a' + shift) % 26) + 'a';

plus it also has the theoretical benefit of working on any character set for which the lowercase letters of the English alphabet are in order and contiguous.

Agree with you 100%. I just think it is easier to explain a formula with numbers, and afterwards easier to read with characters (and a bit more portable).

Originally Posted by laserlight

A switch would guarantee that it works regardless of how those characters are laid out in the character set, but then this advantage is quite theoretical.

Agree too, but I didn't want to bring in portability, as this was just a simple homework assignment.

[mikesnuggets22]

thanks, I'm updating my program now and hopefully it works. in the meantime, I've learned that this is a skeleton of what it's supposed to look like:

You need to be writing a global function to get the shift and another to get the string.

int main()
{
        // Get shift
        int shift = get_shift();

        // Get input string
        std::string input = get_string();

        // Encrypt.  Save the original string
        std::string encrypted;
        encrypt(input, encrypted);

        // Display results
        display(input, encrypted);

......... get_string, decrypt, and display

        return 0;
}

Still thinking about how to do the strings unless it's that simple



Also I'm getting errors from this:

//Loop begin
//Validate input, ie not space etc
en_str[i] = encrypt(current_letter, shift);
//loop end

pa4.cpp:34: error: `i' was not declared in this scope
pa4.cpp:34: error: invalid conversion from `char' to `char*'
pa4.cpp:34: error: initializing argument 1 of `void encrypt(char*, int)'

[monarch_dodra]

Originally Posted by mikesnuggets22

thanks, I'm updating my program now and hopefully it works. in the meantime, I've learned that this is a skeleton of what it's supposed to look like:

You need to be writing a global function to get the shift and another to get the string.

int main()
{
        // Get shift
        int shift = get_shift();

        // Get input string
        std::string input = get_string();

        // Encrypt.  Save the original string
        std::string encrypted;
        encrypt(input, encrypted);

        // Display results
        display(input, encrypted);

......... get_string, decrypt, and display

        return 0;
}

Still thinking about how to do the strings unless it's that simple

yes, this looks much better. Now, you need to implement your functions. It shouldn't be too hard, you already written the code required by get_string in your main. display is basically just cout << input << " " << encrypted << cout.

Originally Posted by mikesnuggets22

Also I'm getting errors from this:

//Loop begin
//Validate input, ie not space etc
en_str[i] = encrypt(current_letter, shift);
//loop end

pa4.cpp:34: error: `i' was not declared in this scope
pa4.cpp:34: error: invalid conversion from `char' to `char*'
pa4.cpp:34: error: initializing argument 1 of `void encrypt(char*, int)'

That's because it was not literal code, and I didn't write the loop. If you choose to not encrypt non-lowercase lettes, you can just do a for(int i=0; i != your_string.size(); ++i). As for the char*/char errors, it is up to you to decide if you want to work on letters at once, or on the entire string.

[laserlight]

Originally Posted by monarch_dodra

While I understand character mapping is environment dependent, I was pretty sure all modern OS followed the ASCII scheme for at least the first 127 symbols, and the rest locale dependent. What do you mean: "no longer in practice"? Do you just mean that there is no guarantee they follow this scheme?

Yes, there is no guarantee.