Mixed Messages: a DnD character generator. Feedback wanted!

Projects Portfolio Project: Mixed Messages
#1

This project was quite easy, but I liked that it was easy! It’s a good first project, with the focus leading up to the project being on the working with version control and how to provide a good README. It’s a nice first project in the path.

It took me about 1,5 hours to complete this project, excluding the time I spent updating Node, setting up VSCode on my computer, and fiddling with keybindings.

The code is available here:

I would like specific feedback on how I generated and combined the messages themselves. Was it a good way to combine them? Could I refine how I randomised each entry? Any feedback regarding this stage would be welcome.

I also began by storing all of my message components in one big object which held each array, but it felt more clunky than useful. How would you have stored the components, if you were me?

#2

Hi there! The DnD idea is a very good one.
I think that your approach to generating the random characters is correct. I have two suggestions:

1. Use const whenever possible

One best practice that you could look into is to always use const for variables that won’t be reassigned.

2. Inline trivial functions

Other thing I’d change is more of a personal preference, but trivial functions (like numGenerator or randEntry) could be inlined in the code, as they don’t hurt readability and they don’t really abstract away meaningful logic. For example:

function startsWithVowel(str) {
    const vowelRegex = /^[aeiou].*/i;
    let matched = str.match(vowelRegex);
    if (matched) {
        return true;
    } else {
        return false;
    }
}

Take this function. If you are advanced enough to use regular expressions I find it a bit overkill to declare a whole function to do a simple match. I would replace it in the function call site like this:

function randCharacter() {
    /.../
    let indefArticle = 'a';
    if (attitude.match(/^[aeiou].*/i))) {
        indefArticle = 'an';
    }

    return `You're ${indefArticle} ${attitude} ${race} ${dndClass}. ${quirk}`;
}

I would also refactor the if into a ternary expression to allow us to declare the variable indefArticle as const, but as I have said, this is more of a stylistic preference.

3. Improve your ReadMe

I think you could be more explicit and give the direct commands to clone, install and run the project. Again, this is more of a personal preference. Related with this point, I think that you should add a ‘start’ script in your package.js as it’s a best practice (if you want you can take a look at my solution’s ReadMe to take ideas).

1 Like
#3

Thank you for your feedback!

I will definitely take a second look and improve my code from your first two points. I am not very experienced with regexps, but I want to learn - which is why I included it.

Regarding the npm and package.js file:

I am not used to using npm. In fact, I believe I managed to set up this package.js file while trying the commands out. I was fiddling with npm and tried to update my node versions etc. Do I even need to have a package for others to run my code?

#4

Hi! As far as I know, the scripts in package.json are just shortcuts for your CLI (launching the app with npm start instead of node myApp.js), so they are not mandatory. The use of package.json is important to manage dependencies as it makes it easier for people to run your code as they can install the dependencies automatically. You can find more information here and some best practices here. I’m learning npm too, so take all that I say with a grain of salt!

Anyway, I’m glad you took some time to read through the comments and happy coding!

1 Like