Code Review Asked by aguywhocodes on December 2, 2021
I made the UI for the hacking minigame from the Fallout series. Could I get some feedback on the main Game.js
component? It’s my first React app.
I’m especially curious as to whether my code is easily testable, and also if it has a lot of clarity, ie. if it’s easy to understand. I am also curious of improvements that can be made to the algorithm. Also, does it follow React best practices?
Game.js
import React from "react";
import CharacterSequence from "./CharacterSequence";
function Game() {
let nbAttempsLeft = 3;
let characters = "./@.!@#$%^&*()-=+><,[]{}";
let words = ["STORY", "SYNOPSIS", "THE", "PLAYER", "CHARACTER", "STUMBLES", "IRRADIATED", "PRESSURE", "ABILITY"];
/**
* Generates a string from filler characters. Ex: "*.!++}/.,.#^"
* @param {*} characters the characters to randomly choose from
* @param {*} length the length of the filler string
*/
function generateFiller(characters, length) {
let filler = "";
for (let i = 0; i < length; i++) {
filler += characters.charAt(Math.floor(Math.random() * characters.length));
}
return filler;
}
/**
* Each row is preceded by 0x${HEXCODE}.
* @param {*} hexStart the decimal number to use as a starting point.
* @param {*} i number of times to multiply increment by.
* @param {*} increment the increment to use when adding to hexStart.
*/
function generateHex(hexStart, i, increment) {
// Each row has a HEX identifier which starts at 61623 (decimal) and increases by 12 every row.
// Ex: 0xF0B7, 0xF0C3, etc.
const hex = `0x${(hexStart + increment * i).toString(16).toLocaleUpperCase()}`;
return hex;
}
/**
* Generates an array of sequences in the Fallout terminal format.
* Ex: 0xEF8B %^ABILITY/.}
* @param {*} amount how many sequences to put in the array.
*/
function generateSequences(amount) {
let sequences = [];
for (let i = 0; i < amount; i++) {
let sequence = `${generateHex(61323, i, 12)} ${generateFiller(characters, 12)}`;
sequences.push(sequence);
}
return sequences;
}
/**
* Randomly adds words from a word list to an array of sequences.
* @param {*} sequences the array of sequences to add words to.
* @param {*} words the word list to choose from.
* @param {*} amount the amount of words to add in the sequences array.
*/
function addWords(sequences, words, amount) {
const lengthOfHex = 7;
for (let i = 0; i < amount; i++) {
// Choose a word in the word list and remove it after (prevent duplicates).
let wordIndex = Math.floor(Math.random() * words.length);
let word = words[wordIndex];
words.splice(wordIndex, 1);
// Choose a random number that will determine where the word starts in the sequence.
// (12 - word.length) is the remaining spaces for filler characters.
let wordStart = Math.floor(Math.random() * (12 - word.length));
// Choose a random sequence to add a word to. TODO: Prevent duplicates.
let index = Math.floor(Math.random() * sequences.length);
sequences[index] = sequences[index].substr(0, wordStart + lengthOfHex) + word + sequences[index].substr(wordStart + word.length + lengthOfHex);
}
}
let sequences = generateSequences(34);
addWords(sequences, words, 9);
return (
<div id="App">
<div id="terminal">
<div className="header">
<p>ROBCO INDUSTRIES (TM) TERMLINK PROTOCOL</p>
<p>ENTER PASSWORD NOW</p>
</div>
<div className="attempts">
<p>{nbAttempsLeft} ATTEMPT(S) LEFT...</p>
</div>
{sequences.map((sequence) => (
<CharacterSequence sequence={`${sequence}`}></CharacterSequence>
))}
</div>
</div>
);
}
export default Game;
```
Game
component, however, doesn't consume any props, so you'll have a single test using the hardcoded magic numbers also defined internally.The suggestion here is to factor these utility functions into a separate file and export, which allows for very easy testing.
My only suggestion here is to utilize the jsDoc notation more effectively by actually declaring the parameter types, and if necessary, define the return type (if your IDE intellisense isn't capable of determining this via static analysis).
My suggestion here is to also move these declarations out of the component and declare them const. Additionally, it appears that nbAttemptsLeft
and sequences
are intended to be component state, so make them component state. Generally speaking, functions shouldn't mutate passed parameters, as is the case with addWords
, so it should return a new array of updated sequence values.
Utility file - these can now all be easily tested in isolation/decoupled from component
/**
* Generate a random integer between 0 inclusive and max exclusive
* @param {number} max maximum random value range, exclusive
*/
export const random = max => Math.floor(Math.random() * max);
/**
* Generates a string from filler characters. Ex: "*.!++}/.,.#^"
* @param {string} characters the characters to randomly choose from
* @param {number} length the length of the filler string
*/
export function generateFiller(characters, length) {
let filler = "";
for (let i = 0; i < length; i++) {
filler += characters.charAt(random(characters.length));
}
return filler;
}
/**
* Each row is preceded by 0x${HEXCODE}.
* @param {number} hexStart the decimal number to use as a starting point.
* @param {number} i number of times to multiply increment by.
* @param {number} increment the increment to use when adding to hexStart.
*/
export function generateHex(hexStart, i, increment) {
// Each row has a HEX identifier which starts at 61623 (decimal) and increases by 12 every row.
// Ex: 0xF0B7, 0xF0C3, etc.
return `0x${(hexStart + increment * i).toString(16).toLocaleUpperCase()}`;
}
/**
* Generates an array of sequences in the Fallout terminal format.
* Ex: 0xEF8B %^ABILITY/.}
* @param {number} amount how many sequences to put in the array.
* @param {string} characters the characters to randomly choose from
*/
function generateSequences(amount, characters) {
const sequences = [];
for (let i = 0; i < amount; i++) {
let sequence = `${generateHex(61323, i, 12)} ${generateFiller(characters, 12)}`;
sequences.push(sequence);
}
return sequences;
}
/**
* Randomly adds words from a word list to an array of sequences.
* @param {string[]} sequences the array of sequences to add words to.
* @param {string[]} words the word list to choose from.
* @param {number} amount the amount of words to add in the sequences array.
* @return {string[]} updated sequences array
*/
export function addWords(sequences, words, amount) {
const lengthOfHex = 7;
// create shallow copy to not mutate passed argument
const updatedSequences = [...sequences];
for (let i = 0; i < amount; i++) {
// Choose a word in the word list and remove it after (prevent duplicates).
const wordIndex = random(words.length);
const word = words[wordIndex];
words.splice(wordIndex, 1);
// Choose a random number that will determine where the word starts in the sequence.
// (12 - word.length) is the remaining spaces for filler characters.
const wordStart = random(12 - word.length);
// Choose a random sequence to add a word to. TODO: Prevent duplicates.
const index = random(sequences.length);
updatedSequences[index] = sequences[index].substring(0, wordStart + lengthOfHex) + word + sequences[index].substring(wordStart + word.length + lengthOfHex);
// NOTE: string::substr is actually a deprecated API and no longer recommended
// use string::substring instead
// return new sequences array
return updatedSequences;
}
}
Game component
import React, { useEffect, useState} from "react";
import CharacterSequence from "./CharacterSequence";
import {
addWords,
generateFiller,
generateHex,
generateSequences,
random,
} from './utils';
const characters = "./@.!@#$%^&*()-=+><,[]{}";
const words = ["STORY", "SYNOPSIS", "THE", "PLAYER", "CHARACTER", "STUMBLES", "IRRADIATED", "PRESSURE", "ABILITY"];
function Game() {
// Number of hacking attempts remaining
const [attemptsLeft, setAttemptsLeft] = useState(3);
// Randomly generated sequence of byte-code
const [sequences, setSequences] = useState(() => {
return addWords(
generateSequences(34),
words,
9
);
});
return (
<div id="App">
<div id="terminal">
<div className="header">
<p>ROBCO INDUSTRIES (TM) TERMLINK PROTOCOL</p>
<p>ENTER PASSWORD NOW</p>
</div>
<div className="attempts">
<p>{attemptsLeft} ATTEMPT(S) LEFT...</p>
</div>
{sequences.map((sequence) => (
<CharacterSequence sequence={`${sequence}`} />
))}
</div>
</div>
);
}
export default Game;
Answered by Drew Reese on December 2, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP