patternjavascriptMinor
Badger's FizzBuzz
Viewed 0 times
badgerfizzbuzzstackoverflow
Problem
I love the badger's song (YouTube), so I wrote a FizzBuzz about it.
The thing is, I've been writing Javascript for years but I'm still not sure about the best practices. So FizzBuzz seemed a simple trial for me to write some code and make it reviewed!
It would be called this way:
Is there something in this JavaScript object that's wrong?
The thing is, I've been writing Javascript for years but I'm still not sure about the best practices. So FizzBuzz seemed a simple trial for me to write some code and make it reviewed!
function FizzBuzz(limit){
var _limit = limit;
var fizzBuzzContent =
[
{"modulo":3, "word":"Badger"},
{"modulo":5, "word":"Mushroom"},
{"modulo":15, "word":"Ooohhh it's a snaaaakkkeeee"}
];
this.calculate = function(){
for(var counter = 1; counter <= _limit; counter++) {
var line = counter.toString();
fizzBuzzContent.forEach(function(e){
if(counter % e["modulo"] === 0){
line = e["word"];
}
});
console.log(line);
}
}
}It would be called this way:
var f = new FizzBuzz(100);
f.calculate();Is there something in this JavaScript object that's wrong?
Solution
A few of things (apologies for repeating points from other answers; I wrote this earlier but didn't have a chance to finish it):
-
Didn't know you could "love" that song. But you can certainly be brainwashed by it. See a doctor.
-
Perhaps
-
I don't think this needs to be a constructor. It could just be a plain function, which would be simpler.
-
Constructor or not, you don't need to copy the
-
-
You don't need to quote keys in an object unless they contain spaces or other non-valid literals. Just writing
And since those keys work as regular property names, you can just write
Though for something so self-contained as this, I'd be fine with just nested array instead. True, you have to write
-
Instead of
-
No need for
So with that, something like this would do the trick too:
-
Didn't know you could "love" that song. But you can certainly be brainwashed by it. See a doctor.
-
Perhaps
BadgerMushroom would be a more appropriate name?-
I don't think this needs to be a constructor. It could just be a plain function, which would be simpler.
-
Constructor or not, you don't need to copy the
limit argument to _limit. It's makes no difference (the argument is effectively already a local variable, and numbers are passed by value, not by reference, so they don't need cloning or stuff like that to avoid side-effects).-
calculate is not the greatest name. Yeah, it does do some calculation, but its main output is log messages. So perhaps print or similar would be a better fit.-
You don't need to quote keys in an object unless they contain spaces or other non-valid literals. Just writing
{ modulo: 5, word: "..." } works fine. (You can quote them, of course, which is why JSON, where quotes are required, is valid JavaScript.)And since those keys work as regular property names, you can just write
e.modulo and e.word - no need for brackets.Though for something so self-contained as this, I'd be fine with just nested array instead. True, you have to write
[0] and [1] instead of .modulo and .word, but it's not as repetitive to define the lookup.-
Instead of
forEach, I'd stick to a simple old for loop - that way, you can break out of it when you get a match.-
No need for
toString on a number. It'll print the same.So with that, something like this would do the trick too:
function badgerMushroom(limit){
var line,
words = [
[15, "Ooohhh it's a snaaaakkkeeee"],
[5, "Mushroom"],
[3, "Badger"]
];
for(var n = 1 ; n <= limit ; n++) {
line = null;
for(var i = 0, l = words.length ; i < l ; i++) {
if(n % words[i][0] === 0) {
line = words[i][1];
break;
}
}
console.log(line || n);
}
}Code Snippets
function badgerMushroom(limit){
var line,
words = [
[15, "Ooohhh it's a snaaaakkkeeee"],
[5, "Mushroom"],
[3, "Badger"]
];
for(var n = 1 ; n <= limit ; n++) {
line = null;
for(var i = 0, l = words.length ; i < l ; i++) {
if(n % words[i][0] === 0) {
line = words[i][1];
break;
}
}
console.log(line || n);
}
}Context
StackExchange Code Review Q#113488, answer score: 5
Revisions (0)
No revisions yet.