Encode a string using Caesar's Cipher

My program is complete and running, but I would like second opinions on how I can make it more efficient. The program prompts the user for the message and the amount s/he would like to shift the letters. For the implementation part, is there another "faster" way to do it instead of having the if else if statements to run through and test my characters?

#include "stdafx.h"
#include  
#include  

using namespace std; 

string encodeCaesarCipher(string str, int shift); 

int main()
{
    string str, encoded_string;  
    int shift; 

    cout > shift; 

    while (shift > shift; 
    }

    encoded_string = encodeCaesarCipher(str, shift); 

    cout << "\nEncoded message: " << encoded_string << "\n\n"; 

    system("PAUSE"); 

    return 0;
}

string encodeCaesarCipher(string str, int shift)
{
    string temp = str; 
    int length; 

    length = (int)temp.length(); 

    for (int i = 0; i < length; i++)
    {
        if (isalpha(temp[i]))
        {
            for (int j = 0; j < shift; j++)
            {
                if (temp[i] == 'z')
                {
                    temp[i] = 'a'; 
                }
                else if (temp[i] == 'Z')
                {
                    temp[i] = 'A'; 
                }
                else 
                {
                    temp[i]++; 
                }
            }
        }
    }

    return temp; 
}

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It is particularly bad to put it into a header file, so please don't do that.

Don't use system("PAUSE")

There are two reasons not to use system("cls") or system("PAUSE"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named PAUSE or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a seperate functions pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example:

void pause() {
    getchar();
}

General portability

This code could be made portable if, in addition to the changes in the previous point, you omit the Windows-only include files #include "stdafx.h".

Use const where practical

The passed str is not modified by encodeCaesarCipher() and so it should be declared as const std::string &. Right now, it's passed by value, so the code is making a useless extra copy.

Use constant string concatenation

Within main the code has several places where successive lines of code do nothing except emit a constant string to std::cout using operator <<. Instead of making separate calls to operator <<, you could call it just once:

std::cout << "You have entered an invalid number. \n" 
    "Please try again. \n" 
    "Enter a proper number for the amount of letter shifting: ";

This only calls << once instead of three times. The compiler automatically concatenates the string literals together.

Use "range-for" to simplify your code

Instead of using an index variable, your for loop could use "range-for":

for (auto &ch : temp)

Consider an alternative approach

Rather than doing all of that calculation for each letter, you could use a simple lookup mechanism instead. Keep two contiguous alphabets in memory and use the plaintext char and rotate value to calculate an offset.

int encode(int ch, int rotate)
{
    const char alphabet[] = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz";
    if (std::islower(ch)) {
        return alphabet[ch-'a'+rotate];
    } else {
        return std::toupper(alphabet[std::tolower(ch)-'a'+rotate]);
    }
}

std::string encodeCaesarCipher(const std::string &str, int shift)
{
    std::string temp{str}; 
    for (auto &ch : temp) {
        if (std::isalpha(ch)) {
            ch = encode(ch, shift);
        }
    }
    return temp; 
}

With some minor rearrangement and a lambda, one could even turn encodeCaesarCipher into a one-line function using std::transform, but I'll leave that for you.

StackExchange Code Review Q#102031, answer score: 6