Issue sorting and reversing an array

  • (2 Pages)
  • +
  • 1
  • 2

17 Replies - 1696 Views - Last Post: 12 November 2015 - 12:35 PM

#1 boomer1204   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 48
  • Joined: 18-April 12

Issue sorting and reversing an array

Posted 05 November 2015 - 02:48 PM

So I understand how to sort and reverse an array but i'm not getting the desired outcome with my app so i'm not sure if i'm not doing it in the right area or what i'm doing wrong.

What's supposed to happen is the dice get rolled and if you get a 6 a blue border goes around it then if there is a 5 rolled a blue border goes around it and then a 4 is rolled a blue border goes around it.

You can't have a 5 until a 6 is rolled, can't have a 4 until a 6 and 5 are rolled and if a 6, 5, 4 are rolled on one turn you can save them all. The code I have now works MOST of the time but not always. When it doesn't work it's because a 5 or 4 will be rolled in the same roll as a 6 but it will be to the left of the 6. If they are to the right it works fine.

This lead me to believe if I sorted the array big to little before checking the problem would be resolved but it's not so i'm clearly doing something wrong here. I will attach the js and the codepen link that i'm using to 'test'.

$(document).ready(function() {
	$(".container").css("width", $(window).width());
	$(".container").css("height", $(window).height());
	$(".dice").height($(".dice").width());
	$("#roll_button").height($(".dice").width());
	
});

var dice1 = new dice(1);
var dice2 = new dice(2);
var dice3 = new dice(3);
var dice4 = new dice(4);
var dice5 = new dice(5);
var diceArray = [dice1, dice2, dice3, dice4, dice5];
var rollButton = document.getElementById('roll_button');
var cargo = 0;
var numOfRolls = 0;
var cargoButton = document.getElementById('cargo');
cargoButton.addEventListener('click', getCargo);



//dice object
function dice(id){
    this.id = id;
    this.currentRoll = 0;
    this.previousRoll = 1;
    this.isSelected = false;
    this.diceImageUrl = "img/dice/dice1.png";
    this.roll = function(){
        this.previousRoll = this.currentRoll;
        this.currentRoll = getRandomRoll(1, 6);
    }
}

//returns an array of all dice that are not currently selected so they can be rolled.
function getRollableDiceList(){
    var tempDiceList = [];
    for(var i = 0; i < diceArray.length; i++){
        if(!diceArray[i].isSelected){
            tempDiceList.push(diceArray[i]);
        }
    }
    return tempDiceList;
}

// gets a random number between min and max (including min and max)
function getRandomRoll(min,max){
    return Math.floor(Math.random() * (max-min + 1) + min);
}

// calls the roll function on each dice
function rollDice(rollableDiceList){
    for(var i = 0; i < rollableDiceList.length; i++){
        rollableDiceList[i].roll();
    }
}

// updates each dice with the new url for the image that corresponds to what their current roll is
function updateDiceImageUrl(){
    for(var i = 0; i < diceArray.length; i++){
        var currentDice = diceArray[i];

        currentDice.diceImageUrl = "http://boomersplayground.com/img/dice/dice" + currentDice.currentRoll + ".png";

        //update div image with img that cooresponds to their current roll
        updateDiceDivImage(currentDice);
    }
}

//Displays the image that matches the roll on each dice
function updateDiceDivImage(currentDice) {
    document.getElementById("dice"+currentDice.id).style.backgroundImage = "url('" + currentDice.diceImageUrl +"')";
}

// returns an array of all
function getNonselectedDice(){
    var tempArray = [];
    for(var i = 0; i < diceArray.length; i++){
        if(!diceArray[i].isSelected){
            tempArray.push(diceArray[i]);
        }
    }
    return tempArray.sort(function(a, B)/>{
      return b - a;
    });
}

function getSelectedDice(){
  var selectedDice = [];
  for(var i = 0; i < diceArray.length; i++){
    if(diceArray[i].isSelected){
      selectedDice.push(diceArray[i]);
    }
  }
  return selectedDice;
}

//boolean variables
var shipExist = false;
var captExist = false;
var crewExist = false;

//checks each dice for ship captain and crew. Auto select the first 6, 5 , 4.
function checkForShipCaptCrew(){
    //array of dice that are not marked selected
    var nonselectedDice = getNonselectedDice();
        
  
    for(var i = 0; i < nonselectedDice.length; i++){
        //temp variable that represents the current dice in the list
        currentDice = nonselectedDice[i];
        if (!shipExist) {
            if (currentDice.currentRoll == 6) {
                shipExist = true;
                currentDice.isSelected = true;
            }
        }
        if (shipExist && !captExist) {
          if (currentDice.currentRoll == 5) {
            captExist = true;
            currentDice.isSelected = true;
          }
        }
        if (shipExist && captExist && !crewExist) {
          if (currentDice.currentRoll == 4) {
              crewExist = true;
              currentDice.isSelected = true;
          }
        } 
    }
}

function addGlow(){
  var selectedDice = getSelectedDice();

  for (var i = 0; i < selectedDice.length; i++){
    var addGlowDice = selectedDice[i];
    addGlowDice.className = ' glowing';

    var element = document.getElementById('dice' + addGlowDice.id);

    element.className = element.className + " glowing";
  }
}

function getCargo(){
  
}

rollButton.addEventListener('click', function(){
        //generate rollable dice list
    if (numOfRolls < 3) {
        var rollableDiceList = getRollableDiceList();

        //roll each dice
        rollDice(rollableDiceList);

        //update dice images
        updateDiceImageUrl();

        getNonselectedDice();
      
        // //auto select first 6, 5, 4 (in that order)
        checkForShipCaptCrew();

        addGlow();
        // //adds a red glow to each dice that is selected
        numOfRolls++;
    }
});



http://codepen.io/bo...zGL?editors=001

Also shared a pic to hopefully help explain a little better.

Thanks in advance guys!!!

Attached image(s)

  • Attached Image


Is This A Good Question/Topic? 0
  • +

Replies To: Issue sorting and reversing an array

#2 ArtificialSoldier   User is online

  • D.I.C Lover
  • member icon

Reputation: 2517
  • View blog
  • Posts: 7,627
  • Joined: 15-January 14

Re: Issue sorting and reversing an array

Posted 05 November 2015 - 02:56 PM

Sorting the array sounds like it would work, or you could also save the rolled values in an array and then just check if a value is in the array. You can check if there is a 6 in any position, a 5 in any position, etc. You could also do multiple passes, loop through once and set variables for which numbers are there and their positions, and then loop again to set the borders based on what is there.
Was This Post Helpful? 1
  • +
  • -

#3 boomer1204   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 48
  • Joined: 18-April 12

Re: Issue sorting and reversing an array

Posted 05 November 2015 - 03:03 PM

Originally I saved the values in an array but the problem is that it's a part of the game rules. You can't save a 5 until a 6 has been rolled (or it's rolled with a 6) and a 4 can't be saved until a 6 and 5 have been rolled or (if they are all rolled in one roll you can save them all). So it was saving the 6 but not the 5 or 4 that was checked before it. That's why I figured if I put the numbers in the array from big to little that would resolve my problem but i'm still experiencing the same problem.

I'm not sure if my code is bad or i'm calling the sort().reverse() in the wrong spot.

Thanks
Was This Post Helpful? 0
  • +
  • -

#4 ArtificialSoldier   User is online

  • D.I.C Lover
  • member icon

Reputation: 2517
  • View blog
  • Posts: 7,627
  • Joined: 15-January 14

Re: Issue sorting and reversing an array

Posted 05 November 2015 - 03:30 PM

You can use indexOf to check the array for any 6, and get the position. Then check it for 5, then 4. You don't have to loop through the array to check, just use indexOf to check if the value is anywhere in the array.
Was This Post Helpful? 1
  • +
  • -

#5 boomer1204   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 48
  • Joined: 18-April 12

Re: Issue sorting and reversing an array

Posted 05 November 2015 - 05:24 PM

Thanks ArtificialSoldier.

I'll take a look at that when I get home and see if I can work with that and figure it out!
Was This Post Helpful? 0
  • +
  • -

#6 boomer1204   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 48
  • Joined: 18-April 12

Re: Issue sorting and reversing an array

Posted 05 November 2015 - 07:34 PM

View PostArtificialSoldier, on 05 November 2015 - 03:30 PM, said:

You can use indexOf to check the array for any 6, and get the position. Then check it for 5, then 4. You don't have to loop through the array to check, just use indexOf to check if the value is anywhere in the array.


So I have been playing around with this for a little bit and it seems like it might be a decent idea. Do you have a rough idea of how I might implement this? I'm trying to figure out if I should get the index first and then check if it's -1 or not, or if I should call it all in the if statement???
Was This Post Helpful? 0
  • +
  • -

#7 ArtificialSoldier   User is online

  • D.I.C Lover
  • member icon

Reputation: 2517
  • View blog
  • Posts: 7,627
  • Joined: 15-January 14

Re: Issue sorting and reversing an array

Posted 06 November 2015 - 10:26 AM

You can just do a couple if statements to check for each number. If the index isn't -1 then you can use the index to set the border on the appropriate die. If you want to highlight every 6 if there are more than 1, then you would just need to loop through the array and highlight each index you find.
Was This Post Helpful? 1
  • +
  • -

#8 boomer1204   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 48
  • Joined: 18-April 12

Re: Issue sorting and reversing an array

Posted 09 November 2015 - 10:21 AM

Thanks!! I'll mess with this over the next couple days. Appreciate the advice alot!!!!
Was This Post Helpful? 0
  • +
  • -

#9 Blindman67   User is offline

  • D.I.C Addict
  • member icon

Reputation: 140
  • View blog
  • Posts: 620
  • Joined: 15-March 14

Re: Issue sorting and reversing an array

Posted 10 November 2015 - 08:01 AM

You have presented a great example of array use in your code.

Your code is good by you have to step back and think a little more about structure and efficiency. Creating new arrays every time you want to do something to items in an array is a little long winded both for you the coder that has to type it all in an make sure there are no bugs, but also for the performance in terms of memory and speed. It may not matter for something like a simple game, but as you move on to bigger and better it does.

THe Dice object is a good move but you have then moved stuff that would naturally be actions of the dice outside and handled that in isolated bits of code. Glow, Image URL, and checking for Captain & stuff. Moving them into the Dice object means less code and a simpler design.

Have a look at MDN Array referance for everything you need to know about arrays.

There are many useful methods in arrays that make for sorter source code (always good) and quicker execution (finally was not so till recently)

Sorts.

// dice list is your dice array
dice.sort(function(a,B)/>/>/>/>{   // a and b are both dice objects with all the properties and method.
    // to sort them you need to sort by there rolled value
    return  a.currentRoll - b.currentRoll;
});


Array iteration.
there is a handy method for arrays called Array.forEach(callback) with callback = function(item, i, array). It goes through each item in the array and calls the callback function. That call back function takes 3 arguments. item is the array item, i is the the items index, and array is the array reference. You don't need to specify each argument if you don't need them.

So when you have an array and you want to roll all the dice in that array (assuming you have added the roll function to the dice object) its very simple, almost a one liner.

// diceToRoll is an array of dice 
diceToRoll.forEach(function(dice){ dice.roll(); }); // call the method roll on each dice.
// there is also the new arrow notation a shorthand way of creating a function
diceToRoll.forEach(dice=>dice.roll); // does the same thing;


So no more for loops to type in over and over.

Filters
Selecting items in an array and creating a new array is also now very simple. Array.filter(callback) does that for you. Callback is in the same form as foreach with item,index,array; The callback returns true if you want that item in the new array of false if you dont.

So to get an array of rollable dice
var rollableDice = diceArray.filter(function(dice){return !dice.isSelected;})
// or with arrow notation where the return is implied
var rollableDice = diceArray.filter(dice=>!dice.isSelected); // same as above;


And there are many more methods available for arrays to make your life easier.

I have taken your code that you posted and rewritten it with all the things I have mentioned plus some. I have not run it so highly likely there are typos, but it is more to give you an idea how you can utilise consistent object design to create smaller code.

As you can see its a lot shorter. Also the Dice object is now where all the action happens.

DOM? wait for it.
Also a warning about your DOM interaction. You had some inside the JQuery.ready function and some outside, adding event handlers was outside. Anything to do with the DOM should happen in or after ready. Also why use JQuery at all if that is all you are using it for. Well up to you, personally I can not wait to see the end of that abbonation.

// add stuff that may change up the top. You never know when you may be forced to change
// domain, directories, class names and more.
var imageDirectory = "http://boomersplayground.com/img/dice/dice"
var glowingClassName = " glowing"
//boolean variables  LOL Stating the obvious
var shipExist = false;
var captExist = false;
var crewExist = false;
var cargo = 0;
var numOfRolls = 0;
// shorter way of creating the dice. see $.ready to see how.
var diceArray = [0,1,2,3,4];


function dice(id){
    this.currentRoll = 0; // only expose the fields you need
    this.previousRoll = 1;
    this.isSelected = false;
    // element is hidden as not needed out side the obbject
    // all functions in this object can use it.
    var element = document.getElementById("dice" + id);  // get it once   
    var glow =  function () {  // hidden function only called in this object
        // warning about hidden functions. They do not have access to "this"
        if(element.className.indexOf(glowingClassName) === -1){ // it may seem trivial but always us === not ==
            element.className += glowingClassName;
        }  
    }   
    // roll is a dice property so do it in the dice object and change the url while here
    this.roll = function () {
        this.previousRoll = this.currentRoll;
        this.currentRoll = Math.floor(Math.random() * 6) + 1; // no point in calling a function so simple
        element.style.backgroundImage = "url('" + imageDirectory + currentDice.currentRoll + ".png')";
    }
    // also this is best done in the dice object
    this.checkForShipCaptCrew = function(){
        // moved the currentRoll to the first test as putting the
        // least likely to be true first in comparisons is more efficient
        if (this.currentRoll === 6 && !shipExist) {
            shipExist = true;
            this.isSelected = true;
        }else
        if (this.currentRoll === 5 && shipExist && !captExist ) {
            captExist = true;
            this.isSelected = true;
        }else
        if (this.currentRoll === 4 && shipExist && captExist && !crewExist) {
            crewExist = true;
            this.isSelected = true;
        } 
        if(this.isSelected){
            glow();  // Add the glow here if needed
        }
    } 
}

function getCargo(){}

function rollDice() {
    if (numOfRolls < 3) {
        var rollableDiceList = diceArray.filter(function (dice) {
            return !dice.isSelected;
        });
        rollableDiceList.forEach(function (dice) {
            dice.roll();
        });
        rollableDiceList.sort(function (a, c) {
            return a.currentRoll - c.currentRoll;
        });
        rollableDiceList.forEach(function (dice) {
            dice.checkForShipCaptCrew();
        });
        numOfRolls++;
    }
}

$(document).ready(function () {
    $(".container").css("width", $(window).width());
    $(".container").css("height", $(window).height());
    $(".dice").height($(".dice").width());
    $("#roll_button").height($(".dice").width());
    // because you are referencing DOM elements in the dice object
    // creating them belongs in here
    diceArray.forEach(function (id) { diceArray[id] = new dice(id + 1); });
    document.getElementById('roll_button').addEventListener('click', rollDice);
    document.getElementById('cargo').addEventListener('click', getCargo);
});




EDIT please note that because of a formatting quirk of D.I.C the sort in the code came out incorrectly. I have reformatted it to be a & c

Hope that helps.

This post has been edited by Blindman67: 10 November 2015 - 11:54 PM

Was This Post Helpful? 1
  • +
  • -

#10 boomer1204   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 48
  • Joined: 18-April 12

Re: Issue sorting and reversing an array

Posted 10 November 2015 - 10:12 AM

HOLY CRAP!! Thank you for taking the time to write all that out. Clearly you can tell i'm new to JS :)/>. As far as the jquery/js mix that's cuz I was pair programming with someone to start and they did that for the size of the "game area". I'm gonna get rid of it I just wanted to get the game working first than I was going to go back.

I did a quick read as i'm at work but I GREATLY appreciate the advice and help. I'm gonna go over it in depth on lunch/after work. Yeah MDN has been a god send for the mess that is my game so far :)/>.

Again greatly appreciate you taking the time and going through instead of just saying here is the answer!!! The little glance I took already seems to make "more sense" as I go over you suggestions!!!

This post has been edited by andrewsw: 10 November 2015 - 01:27 PM
Reason for edit:: Removed previous quote, just press REPLY

Was This Post Helpful? 0
  • +
  • -

#11 felgall   User is offline

  • D.I.C Regular

Reputation: 68
  • View blog
  • Posts: 365
  • Joined: 22-February 14

Re: Issue sorting and reversing an array

Posted 10 November 2015 - 01:23 PM

View PostBlindman67, on 11 November 2015 - 01:01 AM, said:

Also a warning about your DOM interaction. You had some inside the JQuery.ready function and some outside, adding event handlers was outside. Anything to do with the DOM should happen in or after ready. Also why use JQuery at all if that is all you are using it for. Well up to you, personally I can not wait to see the end of that abbonation.


With the Javascript attached in its normal spot just before the </body> tag you don't need to test for the DOM being ready to be accessed as at that point in the page you already know that it is.
Was This Post Helpful? 0
  • +
  • -

#12 andrewsw   User is offline

  • never lube your breaks
  • member icon

Reputation: 6834
  • View blog
  • Posts: 28,372
  • Joined: 12-December 12

Re: Issue sorting and reversing an array

Posted 10 November 2015 - 01:27 PM

@OP You don't have to quote a large previous post, use the Reply button further down the page, or the Fast Reply box.
Was This Post Helpful? 1
  • +
  • -

#13 Blindman67   User is offline

  • D.I.C Addict
  • member icon

Reputation: 140
  • View blog
  • Posts: 620
  • Joined: 15-March 14

Re: Issue sorting and reversing an array

Posted 11 November 2015 - 01:35 AM

@FelGall

How does that help if you are using JQuery UI and many plugins that modify the DOM after it has been loaded? Assuming that the DOM is ready when it reaches the bottom of that page when using JQuery and others will lead to Reference errors.

Why is JQuery itself at the top in the head, does their Javascript have special powers that negates so called "Best Practice"? Why do they advise that your script tags be placed under theirs in the head (without the defer attribute). Why do many polyfills and shims require that they are placed in the head before the DOM. All these thing are just javascript and need to be put in the head because after many years of experience they have found it the best place to put their code.

"Best practice" is understanding how it works, not just play it safe for 90% of cases. Using $.ready ensures that your code will work correctly with JQuery no matter where you put it. Using the script attributes defer and async and starting DOM interaction at the appropriate event makes script placement in the head "Best practice"
Was This Post Helpful? 0
  • +
  • -

#14 felgall   User is offline

  • D.I.C Regular

Reputation: 68
  • View blog
  • Posts: 365
  • Joined: 22-February 14

Re: Issue sorting and reversing an array

Posted 11 November 2015 - 01:52 AM

View PostBlindman67, on 11 November 2015 - 06:35 PM, said:

Why is JQuery itself at the top in the head, does their Javascript have special powers that negates so called "Best Practice"?


No even that is put at the bottom of the page by professionals (otherwise there is a huge SEO penalty from Google for placing a blocking script at the top of the page). It has to come above the scripts that use it but should still be below all of the HTML except for </body></html>

At that point the DOM is fully loaded with the HTML and can be changed by any Javascript that runs there. If you have multiple scripts in separate files where they may try to run before the script files they depend on have finished loading then in that case you may need to delay running the script until the entire page has loaded - then you would need to test for a 'load' event since jQuery will signal ready when just the HTML is loaded in most browsers.
Was This Post Helpful? 0
  • +
  • -

#15 Blindman67   User is offline

  • D.I.C Addict
  • member icon

Reputation: 140
  • View blog
  • Posts: 620
  • Joined: 15-March 14

Re: Issue sorting and reversing an array

Posted 11 November 2015 - 09:36 PM

@Felgall

Quoting Felgall: "(otherwise there is a huge SEO penalty from Google for placing a blocking script at the top of the page)."

This is completely untrue. Take a moment and have a read of google official blog and Official Google Webmaster Central and point out where google states they penalise sites for having script tags in the head. Then while you are there have a look at the page source, specifically the head, make your way pass the dozens of "unprofessionally placed" script tags and what do you find. JQuery, but I guess Google by your standards is being unprofessional.

Quoting Felgall: "No even that is put at the bottom of the page by professionals"
Please do provide a link to a professional site that uses JQuery that has it at the bottom.

Really you should drop your adherence to the bottom of the page scripts. That went out years ago.

Quoting Felgall: "At that point the DOM is fully loaded"
No web sites of old were served up this way, once they left the server they were effectively static documents. This is no longer true ( well I was serving AJAX like pages in 1999) with modern sites relying heavily on extra content that may come from a variety of sources and dependent on user interaction. The DOM can never be considered "full loaded".
Was This Post Helpful? -1
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2