Slot Machine Constructor

This is a fun little program I made of a fruit machine gambler, aka "slot machine". What mistakes am I making? How can I make this better OOP programming, less spaghetti?



var spin = false;

var value1, value2, value3, value4;

var holdFirst, holdSecond, holdThird, holdForth = false;

var first, second, third, forth;

// ***** constructor

function getValue() {
var num = Math.random();

if(num num num num
ul>li {
margin: 20px;
width: 60px;
h2 {
font-size: 60px;
}
}

.active {
background: #ff5a5f !important;
}





JS Bin

• -

• -

• -

• -

Spin


Hold First
Hold Second
Hold Third
Hold Forth




`

-
You have a bug/basic misunderstanding of JS syntax: You can't do chained comparisons in JavaScript, so none of the if(x > num num) num 0.2.

Point is, the code is not doing what you think it's doing.

-
Why card? This isn't a deck of cards. Something like symbol seems more appropriate.

-
new getValue(). There's a lot of weird going on here:

• getValue doesn't sound like a constructor function; it sounds like it's supposed to return a value. So putting new in front of it seems odd.

• If it is a constructor function, the JavaScript convention would be to name it with PascalCase, not camelCase.

• Why doesn't it just return a value? It'd be a lot simpler.

-
Why is getNew called getNew? Why not just spin? Ok, you already have a variable called spin, but that should probably be spinning instead, since that's what describes.

-
Wat?

function startSpining(firstCard, secondCard, thirdCard, forthCard) {

    this.someMethod = function (x) {
        if(spin && !holdFirst){
            x = new getValue();
        }

  };

    if(spin && !holdFirst){
            value1 = new getValue();
            document.getElementById('value1').innerHTML = value1.card;
    }
}

First of all: "start spining". Spellcheck that (it's all over the place). Secondly: This whole chunk is never called as far as I can tell, and I have no idea what the declaration of this.someMethod is supposed to do, or what the function itself is supposed to do (it's supposedly called with an argument, x which you then set to something... and then it does nothing? But of course someMethod is never called anyway). And the indentation is all over the place.

-
Each time you spin, you create a new 10ms interval timer. Click 8 times, and you've got 8 timers running simultaneously, just waiting for spin == true. Probably not what you intend.

-
"Hold forth" should be "Hold fourth". Ironically, "hold forth" is an expression in English. It means "talking lengthily, assertively, or tediously about a subject" (like how I'm doing here!). But that has little to do with slot machines.

-
Why are you loading two versions of jQuery? And why load bootstrap's JS scripts? You don't need them for anything here, as far as I can tell.

-
Why, when you're loading jQuery, aren't you using it for more? As far as I can tell it's only used for one single line: $(el).toggleClass('active');. That's nuts. You're loading a huge library (twice even!) for something that could be handled with 2-3 lines of plain JS.

Meanwhile you're not using jQuery where it'd be useful. For instance, you're setting event handlers in the HTML when jQuery has much more feature-rich event handling. Using onclick attributes is an outmoded technique, and has been for many, many years now. Heck, you're not even using jQuery to find elements, which is perhaps the most basic use of jQuery.

I don't mean to be harsh, but you should probably read up on JavaScript in general. There's a lot here that seems to be just written-until-it-kinda-worked, with little understanding of why it suddenly worked. It seems cobbled together from disparate snippets of code.

And there's little reason to make this object oriented. Or, rather, it is object oriented since you're using the DOM, and the DOM is the Document Object Model. But there's little reason to invent your own constructors.

I'd do something like this:



// Get a random symbol class
function getRandomIconClass() {
var rand = Math.random();

if(rand
.wheel {
width: 25%;
float: left;
text-align: center;
}

.wheel .fa {
display: block;
font-size: 4em;
margin-bottom: 0.5em;
}




Hold




Hold




Hold




Hold




Spin




Addendum: I feel it's worth pointing out that the above doesn't behave like a slot machine (nor does the original implementation). It's more like rolling dice, since each "wheel" can assume any value on each update.

A more "realistic" implementation would be to cycle through the possible values, in order, but make the number of cycles random.



var SYMBOL_CLASSES = ["anchor", "heart", "car", "lemon-o"].map(function (klass) {
return "fa fa-" + klass;
});

$(document).on("click", ".wheel button", function () {
$(this).toggleClass("active").blur().parent().toggleClass("hold");
});

$(document).on("click", "#spin", function () {
var wheels = $(".wheel").not(".hold").find("i"),
wheelCount = wheels.length;

if(wheelCount === 0) {
alert("All wheels are held; there's nothing to spin");
return;
}

var button = $(this).prop("disabled", true).blur();

// keep track of running wheels
var counter = wheelCount;
function stopped() {
if(--counter === 0) {
button.prop("disabled", false);
}
}

// create a bunch of wheel-spinning functions, each slightly offset
var steps = wheels.each(function (i, element) {
var wheel = $(this),
count = 50 + (i *

StackExchange Code Review Q#91990, answer score: 11