Beat Mix Help

[shh003]

Hi, I’ve been working on this beat mix project and I wanted some help reviewing my code. First, I’ll show what I originally typed and then what I simplified it to.

Original:

const toggleDrum = (arrayName, indexNumber) => {
    if (indexNumber >= 0 && indexNumber < 16) {
        if (arrayName === 'kicks') {
                if (kicks[indexNumber] === true) {
                    kicks[indexNumber] = false
                } else {
                    kicks[indexNumber] = true;
                }
            } else if (arrayName === 'snares') {
                if (snares[indexNumber] === true) {
                    snares[indexNumber] = false;
                } else {
                    snares[indexNumber] = true;
                }
            } else if (arrayName === 'hiHats') {
                if (hiHats[indexNumber] === true) {
                    hiHats[indexNumber] = fasle;
                } else {
                    hiHats[indexNumber] = true;
                }
            } else if (arrayName === 'rideCymbals') {
                if (rideCymbals[indexNumber] === true) {
                    rideCymbals[indexNumber] = false;
                } else {
                    rideCymbals[indexNumber] = true;
                }
            }
        }
};

Simplified:

const toggleDrum = (arrayName, indexNumber) => {
    if (arrayName === 'kicks' || arrayName === 'snares' || arrayName === 'hiHats' || arrayName === 'rideCymbals') {
        if (eval(arrayName)[indexNumber] === false) {
            return eval(arrayName)[indexNumber] = true;
        } else if (eval(arrayName)[indexNumber] === true) {
            return eval(arrayName)[indexNumber] = false;
        }
    }
};

I’m much happier with the simplified version, but is there any way to shorten the first if statement so I don’t have to have so many ‘or’ statements? And don’t have to keep typing the string name of the variables?
I’ve also read that using ‘eval’ is a bad idea, any thoughts on alternatives?
Link to problem here

[ionatan]

Is the result going to change or would it be sufficient to run it once?

Do you need an if-statement or is there a co-relation between your result and the value that you want to return?

Are you doing assignment or returning a value? Your previous version doesn’t seem to return anything in particular, why should the new one?

if (arrayName === 'kicks' || arrayName === 'snares' || arrayName === 'hiHats' || arrayName === 'rideCymbals')

If you have many of something, usually you’d keep them in an array.

eval …? very rarely appropriate.

[shh003]

The result is supposed to change when I click on the drum buttons for toggleDrum. The arrays each have 16 values of false, when clicked they should change to true.

The function is supposed to take in a string, ‘kicks’, ‘snares’, etc. and then toggle the kicks, snares, etc. array at the given indexNumber.

I used eval, because this

eval('kicks')[indexNumber] = ...

translates (or so I think) to

kicks[indexNumber] = ...

and alters the array in the way I want, at the given index.

Now, I’ve passed all the tests, but it doesn’t seem that the mixer is working, so I guess this is wrong.

Answers to your questions:
The result is supposed to toggle every time I click on a box.

The if statement is there because kicks[indexNumber] is changing to the opposite of whatever it is (if true then it changes to false, if false then it changes to true).

I believe I’m just doing assignment.

[ionatan]

It’s not wrong (not that part) but it’s overkill to execute a string as javascript code. you should probably be writing code that does what you mean instead.

so instead of executing strings, how about treating them like keys, so that, if you have a string, you can use it to look up an array. Sound familiar?

{
  'kicks': kicks
}

---

If your function is supposed to have an effect, then don’t return a value as well. It gets really difficult to tell what the purpose of a function is when it does a bit of everything.

---

And, if you have true but want false, or vice versa, the relation there is NOT:

a = !a

[ionatan]

So you’d make a mapping from the string identifiers to the arrays… then look it up and make the adjustment.

You might want to skip the condition altogether, because all it’s really doing is suppressing an error that shouldn’t happen, and if something shouldn’t happen then you want an error.

const drums = {
  'kicks': kicks,
  'snares': snares,
  'hiHats': hiHats,
  'rideCymbals': rideCymbals
}

const toggleDrum = (drumName, i) => {
  if (drums.hasOwnProperty(drumName)) {
    const arr = drums[drumName]
    arr[i] = !arr[i]
  }
}

[shh003]

That is what I needed to see, thank you. I’ll give it a shot.

[shh003]

Got it simplified down to this, thank you!

// Drum Arrays
// length of 16, filled with false
let kicks = [];
let snares = [];
let hiHats = [];
let rideCymbals = [];

//gives kicks, snares, hiHats, rideCymbals 16 values of false
const populateArrays = arrayName => {
    for (i = 0; i < 16; i++) {
        arrayName.push(false);
    }
};

populateArrays(kicks);
populateArrays(snares);
populateArrays(hiHats);
populateArrays(rideCymbals);

const drums = {
    'kicks' : kicks,
    'snares' : snares,
    'hiHats' : hiHats,
    'rideCymbals' : rideCymbals,
};

const toggleDrum = (drumName, indexNumber) => {
    if (drums.hasOwnProperty(drumName) && indexNumber >= 0 && indexNumber < 16) {
        const arr = drums[drumName]
        arr[indexNumber] = !arr[indexNumber]
    }
};

const clear = drumName => {
    if (drums.hasOwnProperty(drumName)) {
        for (let i = 0; i < drums[drumName].length; i++) {
            drums[drumName][i] = false;
        }
    }
};

const invert = drumName => {
    if (drums.hasOwnProperty(drumName)) {
        for (let i = 0; i < drums[drumName].length; i++) {
            drums[drumName][i] = !drums[drumName][i];
        }
    }
};

[ionatan]

indexNumber >= 0 && indexNumber < 16

Test the length instead of having magic numbers in there
(and I still don’t think it should be tested at all anything else is invalid input, is the callers fault, should make the program scream and protest)

const invert = drumName => {

doing something to each element is something you’d usually use map for:

[0, 3, 4].map((x) => x+1)  // [1, 4, 5]

and again, don’t agree with the condition

const clear

same thing, map over it

const populateArrays

this could be a more generic function:

given a value and an amount, return an array containing that value repeated that many times, you’d probably name this function replicate

or, this:

> Array(16).fill(false)
[
  false, false, false,
  false, false, false,
  false, false, false,
  false, false, false,
  false, false, false,
  false
]

[ionatan]

So you’d end up with…

const drums = {
  'kicks': Array(16).fill(false),
  'snares': Array(16).fill(false),
  'hiHats': Array(16).fill(false),
  'rideCymbals': Array(16).fill(false)
}

const toggleDrum = (drumName, i) => {
  drums[drumName][i] = !drums[drumName][i]
}

const clear = drumName => {
  drums[drumName].fill(false)
}

const invert = drumName => {
  drums[drumName].map((x, i, arr) => arr[i] = !x)
}

[shh003]

I can’t get that to pass any tests

[ionatan]

When something’s wrong one has to be willing to make observations right? Otherwise it all devolves into wild guesswork, or something like: busted fuse -> replace the whole car

So “tests failed” isn’t the last observation that can be made. One would look at which test, maybe pick the first one that failed, and then, what particular action was it that the test couldn’t carry out, or what was different from what the test expected.

Not saying you particularly need to get that code to fit with the rest, but that’s always going to be how to fix things, to dig in.

[shh003]

I’m trying to figure out the problem, it works when I test it individually, but it won’t load any of the buttons when I open it in a browser.
It says:

1. “before all” hook: load script.js

It won’t load script.js for some reason.

[ionatan]

They have a test for this (missing variables) but the test is broken (guess they didn’t test the tests.)

"before all" hook

that’s something that can be found in the test file

  before('load script.js', (done) => {
    fs.readFile('public/js/script.js', (err, data) => {
      if (err) {
        throw err;
      }
      code = data;
      vm.runInThisContext(code);
      drumArrays = [kicks, snares, hiHats, rideCymbals];
      done();
    });
  });

It’s trying to use those variables, but they’re not there so it crashes

the error message still says what happened:

  1) Beat Mix Problem Set - script.js file
       "before all" hook: load script.js:
     Uncaught ReferenceError: kicks is not defined
      at /root/test/test.js:36:21