patterncppMinor

Simple alphabet scrambler --- usage of vectors

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

codereview cpp stackoverflow c++optimization beginner

alphabetsimplevectorsusagescrambler

Problem

I am currently learning C++, using this book: C++ Primer, 5th Edition. I made this program to help me understand the usage of vectors in C++. I have prior programming experience (mainly Python), and this just seems way too messy to be the right way of doing things. If I'm doing this incorrectly, or inefficiently, then please guide me in the right direction.

The purpose of this program is to take user input, and convert it into random letters (yes, I know that there is a lack of support for #'s and symbols, but that does not matter), somewhat simulating a hashing system.

This isn't homework, and it's not a dire issue. The program runs, but I want to make sure (since I'm new to C++), that I'm doing things in a somewhat correct way.

#include 
#include 
#include 
int main(){
    srand(time(0));
    std::cout > user_password;

    std::cout > choice;

    if (choice == 1) {

        std::vector vuser_pass;

        for ( decltype(user_password.size()) i = 0; i  valpha;

        for ( decltype(alpha.size()) cnt = 0; cnt <= alpha.size(); ++cnt) {

            valpha.push_back(alpha[cnt]);
        } 

        //for ( decltype(valpha.size()) cnt = 0; cnt <= valpha.size(); ++cnt) {
            //std::cout << valpha[cnt] << std::endl;

        //}

        auto limit = vuser_pass.size();
        limit -= 1;

        for ( decltype(vuser_pass.size()) sindex = 0; sindex < limit; ++sindex ) {
            decltype(valpha.size()) vindex = rand() % 25;
            vuser_pass[sindex] = valpha.at(vindex);
        }

        for ( decltype(vuser_pass.size()) i = 0; i < vuser_pass.size(); ++i) {
            std::cout << vuser_pass.at(i);
        }

    }
return 0;
}

Solution

Before discussing whether the algorithm is good or not, there are many small things that you could improve to make the code more readable:

-
Your program won't compile with some compilers if you don't explicitly include ` for rand and srand and for time

.

- 
It is good practice to always fully qualify with

std:: the names of the standard library components you use. Therefore, you should explicitly write std::rand, std::srand and std::time

.

- 
Don't keep commented out comments. They will hardly help you and will probaby just be in the way. If you need to look again at old pieces of code, revision control software are there for you.

- 
This loop:

std::vector vuser_pass;
for ( decltype(user_password.size()) i = 0; i <= user_password.size(); ++i ) {
    char letter_hold = user_password[i];
    vuser_pass.push_back(letter_hold);
}


If I am not mistaken, you are simply asiigning every element of the string

user_password to the vector vuser_password. In this case, please use the std::vector

's constructor which takes two iterators:

std::vector vuser_pass(std::begin(user_password), std::end(user_password));


This do what you need and you will avoid having to manually write a

for loop. Note that it is also valid for the alpha/valpha

 conversion.

- 
That said, do you need

std::vectors at all? You can as well iterator directly on std::string

 since it implements the containers methods.

- 
This loop:

for ( decltype(vuser_pass.size()) i = 0; i < vuser_pass.size(); ++i) {
    std::cout << vuser_pass.at(i);
}


If you use get rid of the vectors and simply use strings, you can directly output the resulting string to

std::cout

:

std::cout << user_password;


- 
You don't need to

return 0; at the end of main. If nothing is returned, the compiler wil automagically add a return 0;

 for you at the end. Not wirting it by yourself amy help to document the fact that you won't ever be returning anything else than

.

- 
You could a range-based

for

 loop to simplify the latest remaining loop:

for (char& c: user_password) {
    c = alpha[rand() % 25];
}


- 
Also,

std::rand and std::srand are kind of advised against in modern C++ since there are safer alternatives in the new

 header. However, the alternative is not that easy to use yet.

std::random_device rd;
std::mt19937 gen(rd());
std::uniform_int_distribution<> dis(0, 25); // will produce ints in [0, 25]


Now, you can create random integers between 0 and 25 by calling

dis(gen)

.

- 
Since

alpha is not meant to be modified, it would be better to make it const`.

So, let's take all of these modifications into account and write your code again:

#include 
#include 
#include 
#include 

int main(){

    // random number generator in [0, 25]
    std::random_device rd;
    std::mt19937 gen(rd());
    std::uniform_int_distribution<> dis(0, 25);

    std::cout > user_password;

    std::cout > choice;

    if (choice == 1) {

        const std::string alpha = "abcdefghijklmnopqrstuvwxyz";

        for (char& c: user_password) {
            c = alpha[dis(gen)];
        }

        std::cout << user_password;
    }
}

Voilà! Your program has been converted to somewhat clean modern C++. Now, all you have to do is to provide more scrambling algorithms and to provide a mechanism to tell the user that they have chosen an invalid option and that they should chose again.

Code Snippets

std::vector<char> vuser_pass;
for ( decltype(user_password.size()) i = 0; i <= user_password.size(); ++i ) {
    char letter_hold = user_password[i];
    vuser_pass.push_back(letter_hold);
}

std::vector<char> vuser_pass(std::begin(user_password), std::end(user_password));

for ( decltype(vuser_pass.size()) i = 0; i < vuser_pass.size(); ++i) {
    std::cout << vuser_pass.at(i);
}

std::cout << user_password;

for (char& c: user_password) {
    c = alpha[rand() % 25];
}

Context

StackExchange Code Review Q#71144, answer score: 6

Revisions (0)

No revisions yet.