6
\$\begingroup\$

In short, the code picks up to 9 different ids from li in jailplayer.

If a hillside, it shall save the value of the id, and then checks if there are more li, up to 9 pcs.

Then I save this in an array, depending on how many ids were found.

This looks like a mess. Can someone help me shorten this code and make it easier to read?

if ($("#jailplayer").children("li").length >= 1) {
                var player1 = $("#jailplayer").find("li:nth-child(1)").attr("id");
                sender = sender + ',';
                sender = sender + player1;

            } else {
                var player1 = '';
            }
            console.log('done one, searching 2');

            if ($("#jailplayer").children("li").length >= 2) {
                var player2 = $("#jailplayer").find("li:nth-child(2)").attr("id");
                sender = sender + ',';
                sender = sender + player2;

            }else {
                var player2 = '';
            }
            if ($("#jailplayer").children("li").length >= 3) {
                var player3 = $("#jailplayer").find("li:nth-child(3)").attr("id");
                sender = sender + ',';
                sender = sender + player3;

            }else {
                var player4 = '';
            }

            if ($("#jailplayer").children("li").length >= 4) {
                var player4 = $("#jailplayer").find("li:nth-child(4)").attr("id");
                sender = sender + ',';
                sender = sender + player4;
            }else {
                var player4 = '';
            }

            if ($("#jailplayer").children("li").length >= 5) {
                var player5 = $("#jailplayer").find("li:nth-child(5)").attr("id");
                sender = sender + ',';
                sender = sender + player5;
            }else {
                var player5 = '';
            }

            if ($("#jailplayer").children("li").length >= 6) {
                var player6 = $("#jailplayer").find("li:nth-child(6)").attr("id");
                sender = sender + ',';
                sender = sender + player6;
            }else {
                var player6 = '';
            }

            if ($("#jailplayer").children("li").length >= 7) {
                var player7 = $("#jailplayer").find("li:nth-child(7)").attr("id");
                sender = sender + ',';
                sender = sender + player7;
            }else {
                var player7 = '';
            }

            if ($("#jailplayer").children("li").length >= 8) {
                var player8 = $("#jailplayer").find("li:nth-child(8)").attr("id");
                sender = sender + ',';
                sender = sender + player8;
                player8 .= ',';

            }else {
                var player8 = '';
            }

            if ($("#jailplayer").children("li").length >= 9) {
                var player9 = $("#jailplayer").find("li:nth-child(9)").attr("id");
                sender = sender + ',';
                sender = sender + player9;
            }else {
                var player9 = '';
            }


            console.log('adding ize');


            var ize =[sender];
\$\endgroup\$
  • 3
    \$\begingroup\$ We can help you shorten and clean up your code, but we could do a much better job if you can explain the objective of the code. What are you trying to accomplish? Also, if you could show a sample of the HTML you are searching through, that would help too. \$\endgroup\$ – jfriend00 Sep 29 '14 at 2:11
8
\$\begingroup\$

It appears that you're just trying to create a list of players from the id values on a list of <li> tags and also build a comma separated list of those ids. If that's the case, you can do the entire code block like this:

var players = $("#jailplayer > li").map(function(index, elem) {
    return this.id;
}).toArray();
var sender = players.join(",");

Note: this doesn't populate variables player1, player2, etc... Instead, it puts the players into an array where they are likely easier to deal with later.


Also, it is unclear what you are trying to do with this:

var ize = [sender];

If you explain the purpose of that, we could offer help on whether that's the right way to to it. Storing an array of a single value that's a comma separated string list of values seems a bit odd which is why I ask.


Key concepts here:

$("#jailplayer > li") - get all the desired <li> tags into a jQuery collection so we can iterate over them.

.map() - iterates over a jQuery collection building a new jQuery object containing the values that the callback returns. Note, the callback will be passed this as each successive element in the original jQuery collection and the id value can be referenced simply as this.id.

.toArray() - get the array out of the jQuery object that was returned so you have just an array, not a jQuery object any more.

.join(",") - take all the elements of the array and join them into a string with the passed in separator between them.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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