Refactoring in Javascript

Question

Here is my code:

var randomColor = ["red", "blue", "green", "#9CBA7F", "yellow", "#BF5FFF"];

function setRandomColor() {
 return randomColor[Math.floor(Math.random() * randomColor.length)];
}

$('.mastermind_master_cpu').each(function() {
 $(this).find('td').each(function() {
   $(this).css("background-color", setRandomColor);
 })
})

As you can see, the mastermind_master_cpu table will randomly fill with different background color. The problem is I have ten different tables and am repeating this every time. Does anyone know how I can go about making this just one function / variable and calling it when needed?

Thanks!

consider codereview.stackexchange.com as a better place for this question. — cookie monster
– cookie monster, Commented Apr 24, 2014 at 16:40
...though I don't know where your repetition is coming from. Looks to me like you're iterating different tables right now. — cookie monster
– cookie monster, Commented Apr 24, 2014 at 16:42
And you seem to be missing the point of passing a function to .css(). The point is that it iterates a collection, but you're passing it to a collection with a single element. Why not $(this).find("td").css("background-color", setRandomColor)? — cookie monster
– cookie monster, Commented Apr 24, 2014 at 16:45

Community · Accepted Answer · 2017-05-23 12:05:09Z

3

Create a class, say random_color, to apply to each table in addition to your current class, like this:

<table class="mastermind_master_cpu random_color">...</table>

Then you can just use this once:

$('.random_color').each(function() {
 $(this).find('td').each(function() {
   $(this).css("background-color", setRandomColor);
 })
})

But as cookie monster points out, this can be done much more succinctly:

$('.random_color td').css("background-color", setRandomColor);

Demonstration

edited May 23, 2017 at 12:05

CommunityBot

11 silver badge

answered Apr 24, 2014 at 16:42

p.s.w.g

150k31 gold badges307 silver badges339 bronze badges

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

4 Comments

cookie monster Over a year ago

Aren't you just doing the same thing but with a different class?

cookie monster Over a year ago

...oh, maybe each table has a unique class right now. That's weird.

p.s.w.g Over a year ago

@cookiemonster That's what I suspect to be the issue.

user3007294 Over a year ago

thanks a ton guys. this works. p.s.w.g was correct, each table had a unique class.

Collectives™ on Stack Overflow

Refactoring in Javascript

1 Answer 1

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related