1
\$\begingroup\$

I'm working on a project of mine, and I've had to write out a fair bit of jQuery. This is a generator and a calculator for some League of Legends related content.

I was wondering if you could see any possible compact-"ness" changes that could be made.

Here is my code. Hopefully I can trust you to take a look at it without taking it.

note: The table is 12x24 broken down into 3x6

var max_points = 30
var spent_points = 0
var total_off = 0
var total_def = 0
var total_utl = 0

$('document').ready(function() {
    $('table.masteries tr.p0 td:nth-child(1)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(2)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(3)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(4)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(5)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(6)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(7)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(8)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(9)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(10)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(11)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");
    $('table.masteries tr.p0 td:nth-child(12)')
        .attr("style","background-image:url('./assets/masteries/mastery0.png')");

    $('table.masteries tr.p0 td').on('click', function(){
    if(spent_points < max_points) {
        if(!this.i){
            this.i = 0;
        }
        s = $(this).find('p').text()
        current_max = parseInt(s.substr(s.length - 1))
        if (
            $(this).is($(':nth-child(1)'))
            || $(this).is($(':nth-child(2)'))
            || $(this).is($(':nth-child(3)'))
            || $(this).is($(':nth-child(4)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_off = total_off + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.offensive').text(total_off);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(5)'))
            || $(this).is($(':nth-child(6)'))
            || $(this).is($(':nth-child(7)'))
            || $(this).is($(':nth-child(8)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_def = total_def + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.defensive').text(total_def);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(9)'))
            || $(this).is($(':nth-child(10)'))
            || $(this).is($(':nth-child(11)'))
            || $(this).is($(':nth-child(12)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_utl = total_utl + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.utility').text(total_utl);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        }
        if(total_off >= 4) {
            $('table.masteries tr.p4 td:nth-child(1)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p4 td:nth-child(2)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p4 td:nth-child(3)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p4 td:nth-child(4)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_def >= 4) {
            $('table.masteries tr.p4 td:nth-child(5)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p4 td:nth-child(6)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p4 td:nth-child(8)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_utl >= 4) {
            $('table.masteries tr.p4 td:nth-child(10)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p4 td:nth-child(11)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p4 td:nth-child(12)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
    }});
    $('table.masteries tr.p4 td').on('click', function(){
    if(spent_points < max_points) {
        if(!this.i){
            this.i = 0;
        }
        s = $(this).find('p').text()
        current_max = parseInt(s.substr(s.length - 1))
        if (
            $(this).is($(':nth-child(1)'))
            || $(this).is($(':nth-child(2)'))
            || $(this).is($(':nth-child(3)'))
            || $(this).is($(':nth-child(4)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_off = total_off + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.offensive').text(total_off);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(5)'))
            || $(this).is($(':nth-child(6)'))
            || $(this).is($(':nth-child(7)'))
            || $(this).is($(':nth-child(8)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_def = total_def + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.defensive').text(total_def);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(9)'))
            || $(this).is($(':nth-child(10)'))
            || $(this).is($(':nth-child(11)'))
            || $(this).is($(':nth-child(12)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_utl = total_utl + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.utility').text(total_utl);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        }
        if(total_off >= 8) {
            $('table.masteries tr.p8 td:nth-child(1)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(2)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(3)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(4)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_def >= 8) {
            $('table.masteries tr.p8 td:nth-child(5)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(6)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(7)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(8)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_utl >= 8) {
            $('table.masteries tr.p8 td:nth-child(9)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(10)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(11)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p8 td:nth-child(12)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
    }});
    $('table.masteries tr.p8 td').on('click', function(){
    if(spent_points < max_points) {
        if(!this.i){
            this.i = 0;
        }
        s = $(this).find('p').text()
        current_max = parseInt(s.substr(s.length - 1))
        if (
            $(this).is($(':nth-child(1)'))
            || $(this).is($(':nth-child(2)'))
            || $(this).is($(':nth-child(3)'))
            || $(this).is($(':nth-child(4)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_off = total_off + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.offensive').text(total_off);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(5)'))
            || $(this).is($(':nth-child(6)'))
            || $(this).is($(':nth-child(7)'))
            || $(this).is($(':nth-child(8)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_def = total_def + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.defensive').text(total_def);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(9)'))
            || $(this).is($(':nth-child(10)'))
            || $(this).is($(':nth-child(11)'))
            || $(this).is($(':nth-child(12)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_utl = total_utl + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.utility').text(total_utl);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        }
        if(total_off >= 12) {
            $('table.masteries tr.p12 td:nth-child(1)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(2)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(3)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(4)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_def >= 12) {
            $('table.masteries tr.p12 td:nth-child(5)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(6)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(7)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(8)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_utl >= 12) {
            $('table.masteries tr.p12 td:nth-child(9)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(10)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(11)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p12 td:nth-child(12)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
    }});
    $('table.masteries tr.p12 td').on('click', function(){
    if(spent_points < max_points) {
        if(!this.i){
            this.i = 0;
        }
        s = $(this).find('p').text()
        current_max = parseInt(s.substr(s.length - 1))
        if (
            $(this).is($(':nth-child(1)'))
            || $(this).is($(':nth-child(2)'))
            || $(this).is($(':nth-child(3)'))
            || $(this).is($(':nth-child(4)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_off = total_off + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.offensive').text(total_off);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(5)'))
            || $(this).is($(':nth-child(6)'))
            || $(this).is($(':nth-child(7)'))
            || $(this).is($(':nth-child(8)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_def = total_def + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.defensive').text(total_def);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(9)'))
            || $(this).is($(':nth-child(10)'))
            || $(this).is($(':nth-child(11)'))
            || $(this).is($(':nth-child(12)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_utl = total_utl + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.utility').text(total_utl);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        }
        if(total_off >= 16) {
            $('table.masteries tr.p16 td:nth-child(1)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p16 td:nth-child(2)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p16 td:nth-child(4)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_def >= 16) {
            $('table.masteries tr.p16 td:nth-child(5)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p16 td:nth-child(6)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p16 td:nth-child(7)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_utl >= 16) {
            $('table.masteries tr.p16 td:nth-child(10)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
            $('table.masteries tr.p16 td:nth-child(11)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
    }});
    $('table.masteries tr.p16 td').on('click', function(){
    if(spent_points < max_points) {
        if(!this.i){
            this.i = 0;
        }
        s = $(this).find('p').text()
        current_max = parseInt(s.substr(s.length - 1))
        if (
            $(this).is($(':nth-child(1)'))
            || $(this).is($(':nth-child(2)'))
            || $(this).is($(':nth-child(4)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_off = total_off + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.offensive').text(total_off);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(5)'))
            || $(this).is($(':nth-child(6)'))
            || $(this).is($(':nth-child(7)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_def = total_def + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.defensive').text(total_def);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if (
            $(this).is($(':nth-child(10)'))
            || $(this).is($(':nth-child(11)'))
        ) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_utl = total_utl + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.utility').text(total_utl);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        }
        if(total_off >= 20) {
            $('table.masteries tr.p20 td:nth-child(2)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_def >= 20) {
            $('table.masteries tr.p20 td:nth-child(6)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
        if(total_utl >= 20) {
            $('table.masteries tr.p20 td:nth-child(10)')
                .attr("style","background-image:url('./assets/masteries/mastery0.png')");
        }
    }});
    $('table.masteries tr.p20 td').on('click', function(){
    if(spent_points < max_points) {
        if(!this.i){
            this.i = 0;
        }
        s = $(this).find('p').text()
        current_max = parseInt(s.substr(s.length - 1))
        if ($(this).is($(':nth-child(2)'))) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_off = total_off + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.offensive').text(total_off);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if ($(this).is($(':nth-child(6)'))) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_def = total_def + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.defensive').text(total_def);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        } else if ($(this).is($(':nth-child(10)'))) {
            if(this.i < current_max) {
                this.i = this.i+1
                total_utl = total_utl + 1
                spent_points = spent_points + 1
                $(this).find('span').text(this.i);
                $('span.utility').text(total_utl);
                $('span.spent').text(spent_points);
            }
            console.log(this.i);
        }
    }});    

});
\$\endgroup\$
  • \$\begingroup\$ Please embed the code you'd like reviewed, as well as its purpose. \$\endgroup\$ – Jamal Aug 12 '14 at 18:25
  • \$\begingroup\$ However, if you really don't want anyone to take it, then embedding would not be advised (so this would have to stay closed). \$\endgroup\$ – Jamal Aug 12 '14 at 18:27
  • \$\begingroup\$ @Jamal - Open it up, Let's trust people... hopefully \$\endgroup\$ – SaiyanToaster Aug 12 '14 at 18:28
  • \$\begingroup\$ is the td has tweleve children?means why u written this 12 times "table.masteries tr.p0" \$\endgroup\$ – paritosh Aug 12 '14 at 18:37
  • \$\begingroup\$ @paritosh the table is 12x24 \$\endgroup\$ – SaiyanToaster Aug 12 '14 at 18:37
4
\$\begingroup\$

A primer for people unfamiliar with League of Legends

This should have been included in OP, but it wasn't. I'll describe the Mastery system here.

League of Legends has a customisable component known as a a collective Mastery Tree. This shares aspects with skill trees or talent trees from other games; it's a hierarchical arrangement of Masteries. Masteries come in three flavours, or trees: Offense, Defense, and Utility, represented by total_off, total_def, and total_utl in OP's code. Players can spend up to 30 total points, and different masteries have different limits on how many points may be spent on them. For example, the Phasewalker mastery has a cap of one point, while a player can spend up to 4 points in the Fury mastery.

In addition to being split up by category, masteries are also separated by depth. Each tree is arranged in six rows. In order to be able to spend points in lower rows, a certain number of points must have been spent in earlier rows in the same tree.

An example of what OP is trying to create can be found here.


  • You've mixed together the trees. This is silly. There's no point in putting Offense masteries together with Utility masteries. Create three separate tables: one for Offense, one for Defense, and one for Utility. By doing so, you'll save yourself from all the pointless if ($(this).is($(":nth-child(1)")) || ... checking.

  • Javascript uses camelCase. I'd rename your variables to maxPoints, etc.

  • CSS should not be set with attr(). Setting CSS with attr is just about the worst way to do it, unless you know what you're doing. This will overwrite all other style changes. Instead, use .css("background-image", "url:(...)").

  • There's no point in modifying each td individually. table.masteries tr.p0 has exactly 12 children, so you can just use $("table.masteries tr.p0 td"). (If for some reason you needed to only select those twelve, then you could use $("... td:nth-child(n + 1):not(:nth-child(n + 13))"), but I'm certain that's not the case here.)

    Here's my rewrite of lines 8 through 31:

    $("table.masteries tr.p0 td").css("background-image", "url('assets/masteries/mastery0.png')");
    
  • Don't create properties in DOM objects to save data. Instead of using this.i, try something like $(this).data("i"). Better yet, make a meaningful name, like $(this).data("pointsSpent").

  • Avoid polluting global scope. Declare local variables with var s before using them.

  • Store jQuery objects instead of continuously creating them. Constructing jQuery objects is not free. Instead, save it with a local variable using var $this = $(this) for later use.

  • Use semicolons consistently. Either use them or omit them. Don't use them in some places and omit them in others.

  • Don't assume console is available. It won't be in IE unless DevTools is open, so avoid using it unless you put var console = console || { log: function(){}, ... }; somewhere in your code.

  • Personally, I think giving rows classes according to their index is pointless; you could just use a :nth-child() selector instead.

  • You aren't using .is() right. Use a selector without constructing a jQuery object: $(this).is(":nth-child(1)").

  • Your way of finding the max points to spend on a mastery is a nightmare to maintain. Rather than scanning some child <p> element, just specify the data on the cell: <td data-maxPoints="4">Fury</td>, or similar.

  • Instead of spent_points = spent_points + 1, you could be doing spent_points++.

  • You are repeating a lot of code that could otherwise be put into a common function.

  • Rather than hard-coding the number of required points, store them in an array or calculate them:

    var pointsRequiredByRow = [0, 4, 8, 12, 16, 20];
    // or just notice that points required = row * 4, where row is zero-based
    

You have a ton to fix. I've listed some things for you to get started on, but a comprehensive review would be way too long. I suggest making some changes and then asking a follow-up question with your revised code.

\$\endgroup\$
  • \$\begingroup\$ Okay thanks for the response, I'll get to fixing some of the stuff \$\endgroup\$ – SaiyanToaster Aug 12 '14 at 21:23

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.