2
\$\begingroup\$

I have to create a 5x5 box using the images, and my first solution was to create 25 variables and print them, which is very inefficient. How can I optimize this? This utilizes cocos2d-x.

auto cudrado0 = Sprite::create("images.png");
cudrado0->setPosition(Vec2(200, 200));
cudrado0->setScale(0.5);
this->addChild(cudrado0, 0);


auto cudrado1 = Sprite::create("images.png");
cudrado1->setPosition(Vec2(200, 275));
cudrado1->setScale(0.5);
this->addChild(cudrado1, 0);

 auto cudrado2 = Sprite::create("images.png");
cudrado2->setPosition(Vec2(200, 350));
cudrado2->setScale(0.5);
this->addChild(cudrado2, 0);

auto cudrado3 = Sprite::create("images.png");
cudrado3->setPosition(Vec2(200, 425));
cudrado3->setScale(0.5);
this->addChild(cudrado3, 0);

auto cudrado4 = Sprite::create("images.png");
cudrado4->setPosition(Vec2(200, 500));
cudrado4->setScale(0.5);
this->addChild(cudrado4, 0);

 auto cudrado5 = Sprite::create("images.png");
cudrado5->setPosition(Vec2(275, 200));
cudrado5->setScale(0.5);
this->addChild(cudrado5, 0);

auto cudrado6 = Sprite::create("images.png");
cudrado6->setPosition(Vec2(350, 200));
cudrado6->setScale(0.5);
this->addChild(cudrado6, 0);

 auto cudrado7 = Sprite::create("images.png");
cudrado7->setPosition(Vec2(425, 200));
cudrado7->setScale(0.5);
this->addChild(cudrado7, 0);

auto cudrado8 = Sprite::create("images.png");
cudrado8->setPosition(Vec2(500, 200));
cudrado8->setScale(0.5);
this->addChild(cudrado8, 0);

  auto cudrado9 = Sprite::create("images.png");
cudrado9->setPosition(Vec2(275, 275));
cudrado9->setScale(0.5);
this->addChild(cudrado9, 0);

 auto cudrado10 = Sprite::create("images.png");
cudrado10->setPosition(Vec2(350, 275));
cudrado10->setScale(0.5);
this->addChild(cudrado10, 0);

auto cudrado11 = Sprite::create("images.png");
cudrado11->setPosition(Vec2(425, 275));
cudrado11->setScale(0.5);
this->addChild(cudrado11, 0);

auto cudrado12 = Sprite::create("images.png");
cudrado12->setPosition(Vec2(500, 275));
cudrado12->setScale(0.5);
this->addChild(cudrado12, 0);

auto cudrado13 = Sprite::create("images.png");
cudrado13->setPosition(Vec2(275, 350));
cudrado13->setScale(0.5);
this->addChild(cudrado13, 0);

auto cudrado14 = Sprite::create("images.png");
cudrado14->setPosition(Vec2(350, 350));
cudrado14->setScale(0.5);
this->addChild(cudrado14, 0);

auto cudrado15 = Sprite::create("images.png");
cudrado15->setPosition(Vec2(425, 350));
cudrado15->setScale(0.5);
this->addChild(cudrado15, 0);

auto cudrado16 = Sprite::create("images.png");
cudrado16->setPosition(Vec2(500, 350));
cudrado16->setScale(0.5);
this->addChild(cudrado16, 0);

auto cudrado17 = Sprite::create("images.png");
cudrado17->setPosition(Vec2(275, 425));
cudrado17->setScale(0.5);
this->addChild(cudrado17, 0);

auto cudrado18 = Sprite::create("images.png");
cudrado18->setPosition(Vec2(350, 425));
cudrado18->setScale(0.5);
this->addChild(cudrado18, 0);

auto cudrado19 = Sprite::create("images.png");
cudrado19->setPosition(Vec2(425, 425));
cudrado19->setScale(0.5);
this->addChild(cudrado19, 0);

auto cudrado20 = Sprite::create("images.png");
cudrado20->setPosition(Vec2(500, 425));
cudrado20->setScale(0.5);
this->addChild(cudrado20, 0);

auto cudrado21 = Sprite::create("images.png");
cudrado21->setPosition(Vec2(275, 500));
cudrado21->setScale(0.5);
this->addChild(cudrado21, 0);

auto cudrado22 = Sprite::create("images.png");
cudrado22->setPosition(Vec2(350, 500));
cudrado22->setScale(0.5);
this->addChild(cudrado22, 0);

auto cudrado23 = Sprite::create("images.png");
cudrado23->setPosition(Vec2(425, 500));
cudrado23->setScale(0.5);
this->addChild(cudrado23, 0);

auto cudrado24 = Sprite::create("images.png");
cudrado24->setPosition(Vec2(500, 500));
cudrado24->setScale(0.5);
this->addChild(cudrado24, 0);
\$\endgroup\$
1
  • \$\begingroup\$ You should look into using a for loop, or possibly a double for loop, and get rid of all unnecessary variables. Whenever you see such repetition of code you should think either loop(s) or methods \$\endgroup\$
    – holroy
    Commented Aug 7, 2015 at 17:58

2 Answers 2

2
\$\begingroup\$

If you have to print a 5x5 box with the a different image or the same you could do something like this. It loops though all the positions that you need and assigns it to a new sprite created. It is more maintaineable, cleaner and easy to read than declaring 25 variables, and it has fewer lines of code.

// Construct the vector with the specified size.
Vec2 size(5, 5);
std::vector<Vec2> positions(size.x * size.y);

// Push back desired positions (you could use C++11 initializer lists or a loop).
// Positions needs to be declared in the same order as the sprites will be
// added with "this->addChild".
positions.push_back(position);
...

float scale(0.5);

// Loop trought the vector and assign values
for (int i = 0; i < size.x * size.y; i++) {
    auto sprite = Sprite::create("theimageyouneed.png");
    sprite->setPosition(positions[i]);
    sprite->setScale(scale);
    this->addChild(sprite, 0);
}
\$\endgroup\$
4
  • \$\begingroup\$ could you give some more explanation of why you would do this and how it makes the code better, please? just edit it into the answer. Thanks \$\endgroup\$
    – Malachi
    Commented Aug 7, 2015 at 18:06
  • \$\begingroup\$ Whilst on the right track this doesn't produce the same as the OPs original code with respect to the positions at least. And why declare sprites when you don't use it? \$\endgroup\$
    – holroy
    Commented Aug 7, 2015 at 18:07
  • \$\begingroup\$ In addition, do you really need to recreate the sprite, or could it be added multiple times? \$\endgroup\$
    – holroy
    Commented Aug 7, 2015 at 18:09
  • \$\begingroup\$ I don't know how the Sprite class works so i can't give a more accurate answer. \$\endgroup\$ Commented Aug 7, 2015 at 18:19
0
\$\begingroup\$

My main issue with your code is the lack of loops, and repetitive code. Given that you can reuse the sprite, and add it multiple times I would go for something like the following untested code:

auto cudrado = Sprite::create("images.png");
cudrado->setScale(0.5);

for (x=0; x<5; x++) {
  for (y=0; y<5; y++) {
     cudrado->setPosition(Vec2(200 + x*75, 200 + y*75));
     this->addChild(cudrado, 0);
  }
}

This could possibly fail if sprites are not reusable, but in that case move the first two lines into the inner loop. Do also note that this doesn't store any of the sprites, but that could be done, if needed, using a double vector/array.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.