Javascript: How would I simplify this code?

Question

I'm new with Javascript and coding in general and I can't figure out a way to simplify this code.

var allCards = ["A1", "A2", "B1","B2", "C1", "C2", "D1", "D2", "E1", "E2", "F1", "F2", "G1", "G2", "H1", "H2"];
var allCardBacks = ["back1","back2","back3","back4","back5","back6","back7","back8","back9","back10","back11","back12","back13","back14","back15","back16"];

there has to be a way to shorten it somehow but I really don't know how to do it. There's also:

function placeCards() {
  setPosition(allCards[0],15, 30);
  setPosition(allCards[1],90, 30);
  setPosition(allCards[2],165, 30);
  setPosition(allCards[3],240, 30);
  setPosition(allCards[4],15, 105);
  setPosition(allCards[5],90, 105);
  setPosition(allCards[6],165, 105);
  setPosition(allCards[7],240, 105);
  setPosition(allCards[8],15, 180);
  setPosition(allCards[9],90, 180);
  setPosition(allCards[10],165, 180);
  setPosition(allCards[11],240, 180);
  setPosition(allCards[12],15, 255);
  setPosition(allCards[13],90, 255);
  setPosition(allCards[14],165, 255);
  setPosition(allCards[15],240, 255);
}

Ples help me. I'm trying to make a memory game but this is too repetitive.

You will need to learn how to use a for loop.

castletheperson
– castletheperson

2017-04-09 23:16:26 +00:00
Commented Apr 9, 2017 at 23:16 — castletheperson
– castletheperson, Commented Apr 9, 2017 at 23:16

Aluan Haddad · Accepted Answer · 2017-04-10 06:42:13Z

5

Nested for loops

function placeCards() {
  var cnt = 0;
  var y;
  var x;
  for (y = 30; y <= 255; y += 75) {
    for (x = 15; x <= 240; x += 75) {
        setPosition(allCards[cnt], x, y);
        cnt++;
    }
  }
}

edited Apr 10, 2017 at 6:42

Aluan Haddad

32.1k10 gold badges83 silver badges95 bronze badges

answered Apr 9, 2017 at 23:17

epascarello

208k20 gold badges206 silver badges246 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Aluan Haddad Over a year ago

This is true, but either use a block scoped declarator, like let, or declare all the vars at the top of the function. The OP is a self-proclaimed newbie and should not be misled

A. L Over a year ago

x would be 15 for all instances no?

A. L Over a year ago

Now you have an infinite loop in the inner loop. I think it's best to actually make a snippet and do runs so you prevent these kind of mistakes :)

VTr Over a year ago

@AluanHaddad Please respect, do not change the author's code if it does not wrong. You changed my answer from right to wrong, and I worry about that. Thanks.

Aluan Haddad Over a year ago

all vars are hoisted. Either use a let declaration or declare them at the top of the function.

VTr · Accepted Answer · 2017-04-10 06:52:38Z

2

Another solution without litmited cards length.

var allCards = ["A1", "A2", "B1","B2", "C1", "C2", "D1", "D2", "E1", "E2", "F1", "F2", "G1", "G2", "H1", "H2"];
var allCardBacks = ["back1","back2","back3","back4","back5","back6","back7","back8","back9","back10","back11","back12","back13","back14","back15","back16"];

function placeCards() {
    for (var i = 0, l = allCards.length; i < l; i += 4) {
        for (var j = 0; j < 4; j++) {
            setPosition(allCards[i + j], 15 + j * 75, 30 + i / 4 * 75);
        }
    }
}

function setPosition(card, x, y) {
    console.log(card, x, y);
}

placeCards();

edited Apr 10, 2017 at 6:52

answered Apr 9, 2017 at 23:27

VTr

7183 silver badges11 bronze badges

1 Comment

Aluan Haddad Over a year ago

You realize that j is not scoped to the inner loop, right?

Aluan Haddad · Accepted Answer · 2017-04-10 06:40:04Z

A simple and effective way to solve problems in JavaScript is to decompose them into small, focused functions that each deal with a specific aspect of a problem.

In your case, you want to calculate a cards position based on its index, increasing the horizontal and vertical distance by 75 units every 4 cards.

Try something like

/**
 * Calculates a horizontal and vertical position based on an index.
 * @param cardIndex The index of a card within
 * @returns an object with `x` and `y` properties corresponding to the horizontal and 
 * vertical placement offset that should be used for the specified cardIndex.
 */
function positionFromIndex(cardIndex) {
  return {
    x: 15 + (cardIndex % 4) * 75,
    y: 30 + Math.floor(cardIndex / 4) * 75
  };
}

/**
 * A stub version of setPosition that simply logs its arguments.
 */
function setPosition(card, x, y) {
  console.log(card, x, y);
}

/**
 * Places a card based on its associated array index.
 * @param card a string representation of a card.
 * @param index the array index associated with the specified card.
 */
function placeCard(card, index) {
  const position = positionFromIndex(index);
  setPosition(card, position.x, position.y);
}

/**
 * lays out an array of cards based on their relative order, moving horizontally 
 * and then vertically.
 */
function placeCards(cards) {
  // Arrays have a `forEach` method that takes a function and calls it for each 
  // element in the array, passing the element and also its index.
  // since our `placeCard` function takes a card and its index we can use it easily
  cards.forEach(function (card, cardIndex) {
    placeCard(card, cardIndex);
  });
  
}


const allCards = [
  "A1", "A2", "B1","B2", "C1", "C2", "D1", "D2", 
  "E1", "E2", "F1", "F2", "G1", "G2", "H1", "H2"
];

placeCards(allCards);

Tip:

In your example code, you have two arrays, although only one is used, your fully program likely uses both. Instead of maintaining corresponding properties in parallelly indexed arrays, which can easily get out of sync and lead to tough bugs, consider declaring allCards as an array of objects like so:

const allCards = [
  { face: "A1", back: "back1" },
  { face: "A2", back: "back2" },
  { face: "B1", back: "back3" },
  { face: "B2", back: "back4" },
  { face: "C1", back: "back5" },
  { face: "C2", back: "back6" },
  { face: "D1", back: "back7" },
  { face: "D2", back: "back8" },
  { face: "E1", back: "back9" },
  { face: "E2", back: "back10" },
  { face: "F1", back: "back11" },
  { face: "F2", back: "back12" },
  { face: "G1", back: "back13" },
  { face: "G2", back: "back14" },
  { face: "H1", back: "back15" },
  { face: "H2", back: "back16" }
];

epascarello · Accepted Answer · 2017-04-09 23:20:52Z

-6

You may try:

function placeCards()
{

  for(var i=0;i<=allCards.length;i++){

    var x,y;
    setPosition(allCards[i],x, y);

  }

}

edited Apr 9, 2017 at 23:20

epascarello

208k20 gold badges206 silver badges246 bronze badges

answered Apr 9, 2017 at 23:18

Kizito Steven

178 bronze badges

5 Comments

epascarello Over a year ago

So how would they calculate x and y?

Kizito Steven Over a year ago

Okay..Way To Go epascarello...Yours aint bad unless u know what exactly the x and y values are needed for. Think Twice Brother

epascarello Over a year ago

I did not down vote you. I fixed your answer so it was actually readable. Your answer right now sets all positions to undefined. Probably why you are getting down votes.

A. L Over a year ago

your answer is incomplete which is why people downvoted. And the OP seems to know EXACTLY what the x and y values are, therefore your argument is moot. And in the event that the OP had an unlimited/unknown length. This answer does not remotely even help to achieve the final answer

Kizito Steven Over a year ago

I think Pig Ball has clear Content..Ts generally the right way to do this.

Collectives™ on Stack Overflow

Javascript: How would I simplify this code?

4 Answers 4

5 Comments

1 Comment

Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

5 Comments

1 Comment

Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related