Fewer Copies
#Fewer Copies
LookingLooking at this, I see several places where data is copied and probably doesn't need to be. For example, I don't think you need to convert from a std::string to a std::vector as strings handle all the things you do with the vector natively. You can reverse the string directly by using operator[], at(), or iterators. Which brings me to my next point.
Iterators
#Iterators
GenerallyGenerally, in C++ it's better to use iterators when possible. Rather than getting the length of the string, then iterating over it manually, you simply call the object's begin() method (which returns an iterator), compare it to the result of the object's end() method, and increment it as you go. In this case we want to go from back to front, so we use a reverse iterator by calling rbegin() (short for reverse iterator begin) and comparing to rend() (reverse iterator end). So to reverse a string you could do this:
Iterate Less Often
#Iterate Less Often
InIn your cleanString() function you iterate over the input string twice. You could do it only once by doing something like this:
Use const
#Use const
InIn many of your methods you aren't changing the input value. If the argument isn't modified by the function (and isn't passed to another function which will modify it), you should mark it as const. This tells the compiler that it won't change, and it tells other people reading the code that it won't change. That makes it easier for the compiler to perform optimizations, and makes it easier for readers to reason about the code.
Use References Where Appropriate
#Use References Where Appropriate
SinceSince you've marked this with the "beginner" tag, you might not yet have learned about references. Normally when you pass an argument to a function you are making a copy of that value to use within the function. With a reference, you avoid that copy and instead pass information about where the data is so it can be looked up. The code you write inside the function will look the same, but you'll be working on the original value rather than a copy. If you mark it as const then you'll only be able to read it, and not write to it, so it won't get changed, but you can still access it.
#Fewer Copies
Looking at this, I see several places where data is copied and probably doesn't need to be. For example, I don't think you need to convert from a std::string to a std::vector as strings handle all the things you do with the vector natively. You can reverse the string directly by using operator[], at(), or iterators. Which brings me to my next point.
#Iterators
Generally, in C++ it's better to use iterators when possible. Rather than getting the length of the string, then iterating over it manually, you simply call the object's begin() method (which returns an iterator), compare it to the result of the object's end() method, and increment it as you go. In this case we want to go from back to front, so we use a reverse iterator by calling rbegin() (short for reverse iterator begin) and comparing to rend() (reverse iterator end). So to reverse a string you could do this:
#Iterate Less Often
In your cleanString() function you iterate over the input string twice. You could do it only once by doing something like this:
#Use const
In many of your methods you aren't changing the input value. If the argument isn't modified by the function (and isn't passed to another function which will modify it), you should mark it as const. This tells the compiler that it won't change, and it tells other people reading the code that it won't change. That makes it easier for the compiler to perform optimizations, and makes it easier for readers to reason about the code.
#Use References Where Appropriate
Since you've marked this with the "beginner" tag, you might not yet have learned about references. Normally when you pass an argument to a function you are making a copy of that value to use within the function. With a reference, you avoid that copy and instead pass information about where the data is so it can be looked up. The code you write inside the function will look the same, but you'll be working on the original value rather than a copy. If you mark it as const then you'll only be able to read it, and not write to it, so it won't get changed, but you can still access it.
Fewer Copies
Looking at this, I see several places where data is copied and probably doesn't need to be. For example, I don't think you need to convert from a std::string to a std::vector as strings handle all the things you do with the vector natively. You can reverse the string directly by using operator[], at(), or iterators. Which brings me to my next point.
Iterators
Generally, in C++ it's better to use iterators when possible. Rather than getting the length of the string, then iterating over it manually, you simply call the object's begin() method (which returns an iterator), compare it to the result of the object's end() method, and increment it as you go. In this case we want to go from back to front, so we use a reverse iterator by calling rbegin() (short for reverse iterator begin) and comparing to rend() (reverse iterator end). So to reverse a string you could do this:
Iterate Less Often
In your cleanString() function you iterate over the input string twice. You could do it only once by doing something like this:
Use const
In many of your methods you aren't changing the input value. If the argument isn't modified by the function (and isn't passed to another function which will modify it), you should mark it as const. This tells the compiler that it won't change, and it tells other people reading the code that it won't change. That makes it easier for the compiler to perform optimizations, and makes it easier for readers to reason about the code.
Use References Where Appropriate
Since you've marked this with the "beginner" tag, you might not yet have learned about references. Normally when you pass an argument to a function you are making a copy of that value to use within the function. With a reference, you avoid that copy and instead pass information about where the data is so it can be looked up. The code you write inside the function will look the same, but you'll be working on the original value rather than a copy. If you mark it as const then you'll only be able to read it, and not write to it, so it won't get changed, but you can still access it.
I think you're on the right track with this. I can see a few things that could be cleaned up.
#Fewer Copies
Looking at this, I see several places where data is copied and probably doesn't need to be. For example, I don't think you need to convert from a std::string to a std::vector as strings handle all the things you do with the vector natively. You can reverse the string directly by using operator[], at(), or iterators. Which brings me to my next point.
#Iterators
Generally, in C++ it's better to use iterators when possible. Rather than getting the length of the string, then iterating over it manually, you simply call the object's begin() method (which returns an iterator), compare it to the result of the object's end() method, and increment it as you go. In this case we want to go from back to front, so we use a reverse iterator by calling rbegin() (short for reverse iterator begin) and comparing to rend() (reverse iterator end). So to reverse a string you could do this:
std::string reverseString(const std::string& inputStr)
{
std::string result;
for (auto nextChar = inputStr.rbegin(); nextChar < inputStr.rend(); ++nextChar)
{
result.append(1, *nextChar); // Could also use opertor+=() here
}
return result;
}
By doing that, we've collapsed 3 functions into 1!
#Iterate Less Often
In your cleanString() function you iterate over the input string twice. You could do it only once by doing something like this:
std::string cleanString(std::string inputString) {
std::locale loc;
std::string result;
for (auto nextChar = inputString.begin(); nextChar < inputString.end(); ++nextChar)
{
if (not std::isspace(*nextChar))
{
result += std::tolower(*nextChar);
}
}
return result;
}
Here we build up the result string by adding the values from the input, but only if they aren't spaces.
#Use const
In many of your methods you aren't changing the input value. If the argument isn't modified by the function (and isn't passed to another function which will modify it), you should mark it as const. This tells the compiler that it won't change, and it tells other people reading the code that it won't change. That makes it easier for the compiler to perform optimizations, and makes it easier for readers to reason about the code.
#Use References Where Appropriate
Since you've marked this with the "beginner" tag, you might not yet have learned about references. Normally when you pass an argument to a function you are making a copy of that value to use within the function. With a reference, you avoid that copy and instead pass information about where the data is so it can be looked up. The code you write inside the function will look the same, but you'll be working on the original value rather than a copy. If you mark it as const then you'll only be able to read it, and not write to it, so it won't get changed, but you can still access it.
For objects like strings that might be very large, avoiding such a copy can make your code more efficient. This probably isn't a big problem with this code, but is good to learn about for the future.