A few fairly minor things:
Constants for the characters
Instead of using the constants, you can just use a character inline:
if(input[index] >= 'a' && input[index] <= 'm')
It's worth noting though that a literal character will be a char
rather than an int
like your constants are (won't affect anything in this program though).
using namespace std;
It's fine in short programs like this, but as you hinted that you're fairly new to C++, I wanted to mention that this can be a bad habit to get into. In particular, in large applications with many different segments of code, this can cause naming clashes.
This question on SO addresses the problems it can cause.
Naming
I would probably call rot
, rot13
since there are other kinds of rotations. This is mainly me just being overly picky though :).
Strange Character For Space
The space character is showing up as a crazy character in my terminal screen, I'm assuming it's because of character encoding, although I could be wrong.
rot13 traditionally only operates on alpha characters (a-z and A-Z), so your space characters should not be changing.
I believe your program has a bug in it:
else if(input[index] <= UPPER_N && input[index] <= UPPER_Z)
If this is not a bug, you should know that this will be always be true if input[index] <= UPPER_N
, so really there's no point in the second conditional.
Anyway, I think what's happening is that the space character (and anything under 'N') is falling into this block when it shouldn't be.
Space is ASCII 32, so it's getting changed to 19, which is not a printable character.
while
vs for
Your loop seems to fit the construct of a for
loop very well, so I might consider using that:
for (int inputSize = input.size(), index = 0; index != inputSize; ++index) {
}
This comes down to personal preference though.
You should usually try to match the return type of a method
std::string::size()
doesn't return an int; it returns a std::string::size_type
which is always (in every implementation I've ever seen anyway), a size_t
(which is in turn an unsigned 32 or 64 bit integer depending on the platform and compiler).
Anyway, when working with standard containers (or really APIs in general), unless you have a reason to use a different type, I would try to stick with the return type specified. For example:
for (std::string::size_type len = input.size(), idx = 0; idx != len; ++idx) {
//input[idx] = ...;
}
A potential implementation
I suspect that the if-else tree can be simplified a bit using isalpha
and family, but this is what I might implement your algorithm like;
std::string rot13(std::string input) {
for (std::string::size_type len = input.length(), idx = 0; idx != len; ++idx) {
if (input[idx] >= 'a' && input[idx] <= 'm') {
input[idx] = input[index] + 13;
} else if (input[idx] >= 'n' && input[idx] <= 'z') {
input[idx] = input[idx] - 13;
} else if(input[idx] >= 'A' && input[idx] <= 'M') {
input[index] = input[index] + 13;
} else if(input[idx] >= 'N' && input[idx] <= 'Z') {
input[index] = input[index] - 13;
}
}
return input;
}
To get a little carried away for a moment, you could also implement it using iterators. Using iterators would allow you to apply it to any container that implements iterators (all of the standard containers and plain pointers).
template <typename Iter>
void rot13(Iter begin, const Iter& end) {
while (begin != end) {
//Doesn't need to be here, but I'm lazy and don't like
//typing *begin over and over again.
char& c = *begin;
if (c >= 'a' && c <= 'm') {
c += 13;
} else if (c >= 'n' && c <= 'z') {
c -= 13;
} else if (c >= 'A' && c <= 'M') {
c += 13;
} else if (c >= 'N' && c <= 'Z') {
c -= 13;
}
++begin;
}
}
Note that this modifies a container in place rather than creating a copy:
char str[] = "Hello World";
rot13(str, str + strlen(str));
std::cout << str << std::endl;
rot13(str, str + strlen(str));
std::cout << str << std::endl;
Would output:
Uryyb Jbeyq
Hello World
You could of course modify it to operate on a copy instead though.