HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Brute force algorithm for printing all possible alphanumeric combinations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
forceallcombinationsalphanumericalgorithmpossibleprintingforbrute

Problem

I'm still very much a beginner, so I don't really know much about best practices or the speed of any particular C functions.

You'll notice that I included a "cs50" library. This is from a MOOC I'm taking, all it does in this program is allow me to use their "bool" data type. I realize I don't need to delegate all of this code to so many functions, but this is part of a bigger problem set from said MOOC, where I am to crack a password, so I thought to make the functions now rather than later.

The program loops through and prints all possible iterations of alphanumeric combinations (no uppercase) from length 1-8. The algorithm in its current state is very slow. I've also never run it to the very end:

#include        //GetString, booleans, and string datatype
#include       //printf
#include      //strlen

#define LOOP_MAX 8

bool BruteForceIntializer();
int looper (int amt, int count);

char engWord[9];
char alphaNum[36] = {"0123456789abcdefghijklmnopqrstuvwxyz"};
int maxSize = sizeof(alphaNum);

int main(void)
{
    if (BruteForceIntializer() == true)
        printf("Success.\n");
    else
        printf("Failure.\n");
}

bool BruteForceIntializer()
{  
    int count;
    for (int amt = 0; amt < LOOP_MAX; amt++)
    {
        memset(engWord, '\0', 9);
        count = 0;
        if (looper(amt, count) == 0)
        {
            continue;
        }     
    }
    return true; //only exits loop if all chars are looped successfully
}

int looper(int amt, int count)
{
    for(int i = 0; i < maxSize; i++)
    {
        engWord[count] = alphaNum[i];
        if (count == amt) //check if we're at the last character
        {
            printf("%s\n", engWord);
            if (i == (maxSize-1))
                return 0;
        }
        if (count != amt)
        {
            if (looper(amt, count+1) == 0)
                continue;
        }
    }
    return 1;
}

Solution

I must say that this looks pretty clean for a beginner. There are no obvious problems with indentation and whitespace (common with beginners), which is a great start. This especially makes it easier for others to read your code and for reviewers to focus on the more important aspects.

-
In most cases, don't use global variables:

char engWord[9];
char alphaNum[36] = {"0123456789abcdefghijklmnopqrstuvwxyz"};
int maxSize = sizeof(alphaNum);


This is highly discouraged because they're exposed to all the code and can be modified from anywhere, making maintenance and debugging much more difficult. Instead, you can just put them in looper() since only that function uses them. That way, if they're modified by accident, you'll know where to look.

In addition, alphaNum and maxSize should be const since they're not meant to be modified:

const char alphaNum[36] = {"0123456789abcdefghijklmnopqrstuvwxyz"};
const int maxSize = sizeof(alphaNum);


-
Some other issues seem to be with the naming:

-
There is one inconsistency: the naming convention of both non-main functions. It's common for functions in C to start with a lowercase letter (either in camelCase or snake_case), so BruteForceInitializer() should be renamed as such.

-
It doesn't seem to make sense for BruteForceInitializer() to return bool if it's actually initializing something. It looks like this function is doing most of the work anyway. You could consider renaming this function in the verb form, which is common with functions (since they're performing a task). It'll also help the check in main() to make more sense.

-
The name looper() doesn't quite seem clear, especially with the values it returns. Based on the one comment in there, perhaps this function should also return bool.

Code Snippets

char engWord[9];
char alphaNum[36] = {"0123456789abcdefghijklmnopqrstuvwxyz"};
int maxSize = sizeof(alphaNum);
const char alphaNum[36] = {"0123456789abcdefghijklmnopqrstuvwxyz"};
const int maxSize = sizeof(alphaNum);

Context

StackExchange Code Review Q#75591, answer score: 3

Revisions (0)

No revisions yet.