1
\$\begingroup\$

My HTML has three divs, each id tagged with the logic behind my object (ma/shadow/traps) which contain 10 div each (every icons), also ID-tagged according to the object. On click, I'm gathering the ID and the parent ID to traverse the object and find the value.

It's only possible to raise the value if the pre-requirement is not 0, or if the pre-requirement is false (no pre-req). My problem comes when there are 2 pre-requirements which need to be checked. I check the first one, and if the first meets the pre-req, I check if a second one exists, and check it.

There has to be a better way of checking on variables than this nightmare of ifs. It works, but I don't think it's good code at ALL. I'm not looking for a full copy/paste code here, but more for a more efficient logic on how to do what I'm already doing.

$(function () {
var basemin = 0;
var basemax = 20;
var skill = {
    ma:{
    //sample object
    },
    shadow:{
        clawMastery:{
            base:0,
            preReq:false
        }, psychicHammer:{
            base:0,
            preReq:false
        }, burstOfSpeed:{
            base:0,
            preReq:'clawMastery'
        }, weaponBlock:{
            base:0,
            preReq:'clawMastery'
        }, cloakOfShadows:{
            base:0,
            preReq:'psychicHammer'
        }, fade:{
            base:0,
            preReq:'burstOfSpeed'
        }, shadowWarrior:{
            base:0,
            preReq:'cloakOfShadows',
            preReq2:'weaponBlock'
        }, mindBlast:{
            base:0,
            preReq:'cloakOfShadows'
        }, venom:{
            base:0,
            preReq:'fade'
        }, shadowMaster:{
            base:0,
            preReq:'shadowWarrior'
        }
    },
    traps:{
    //sample object
    }
};
$('#tree div').bind("contextmenu", function (e) {
    e.preventDefault();
});
$('#tree .tab div').mousedown(function (e) {
    var $this = $(this);
    $this.attr('unselectable', 'on').css('UserSelect', 'none').css('MozUserSelect', 'none');
    var $tab = $this.parent().attr("id");
    var $skill = $this.attr("id");
    function checkPreReq(e) {
        var $preReq = skill[$tab][e]['preReq'];
        var $preReq2 = skill[$tab][e]['preReq2'];
        alert($preReq);
        if ($preReq === false) {
            alert('no pre-req');
            return true;
        } else if ($preReq !== false) {
            alert('pre-req1 exists');
            if(skill[$tab][$preReq]['base'] > 0) {
                alert('pre-req1 higher than 0');
                if ($preReq2 === undefined) {
                    alert('no second pre-req, go on');
                    return true;
                } else {
                    alert('check second pre-req value');
                    if (skill[$tab][$preReq2]['base'] > 0) {
                        alert('pre-req2 higher than 0');
                        return true;
                    } else {
                        alert('pre-req2 not met (0)');
                        return false;
                    }
                }
            } else {
                alert('pre-req1 not met (0)');
                return false;
            }
        } else {
            alert('else');
            return false
        }
    }
    if (e.which == 1) {
        //leftclick
        if (skill[$tab][$skill]['base'] < basemax && checkPreReq($skill)) {
            skill[$tab][$skill]['base'] += 1;
            //$rem_skills -= 1;
        }
    } else if (e.which == 3) {
        //rightclick
        if (skill[$tab][$skill]['base'] > basemin) {
            skill[$tab][$skill]['base'] -= 1;
            //$rem_skills += 1;
        }
    }
    $this.find(".lvl").text(skill[$tab][$skill]['base']);
    //$("#output").find(".rem_skills").text($rem_skills);
});
});
\$\endgroup\$

3 Answers 3

5
\$\begingroup\$

If there are zero, one, two or possible more prerequirements, generalize their storage. Do not use multiple properties and nested if-else-statements, but an array and a loop.

The function should then look like this:

function checkPreReq(e) {
    var preReqs = skill[tab][e].preReqs;
    if (!preReqs)
        return true;
    for (var i=0; i<preReqs.length; i++) {
        var preReq = preReqs[i];
        if (skill[tab][preReq].base <= 0)
            // failed at least this preReq
            return false;
    }
    // passed all requirements
    return true;
}
\$\endgroup\$
6
  • 1
    \$\begingroup\$ +1 Yes, same comment as for RoneRackal's answer. Also, looping with $.each() would simplify the code slightly, but that's optional. \$\endgroup\$ Commented Feb 3, 2013 at 23:15
  • \$\begingroup\$ I cannot seem to extract the array, preReqs thus always returns true (being false/undefined), my array now looks like shadowWarrior:{ base:0, preReqs:{ 1:'cloakOfShadows', 2:'weaponBlock' } } \$\endgroup\$
    – user1732521
    Commented Feb 3, 2013 at 23:41
  • \$\begingroup\$ @user1732521: That's not an array. Use { base:0, preReqs: ['cloakOfShadows', 'weaponBlock'] } \$\endgroup\$
    – Bergi
    Commented Feb 3, 2013 at 23:47
  • \$\begingroup\$ @Beetroot-Beetroot: I can't see why $.each would simplify anything. If at all, you might use .every \$\endgroup\$
    – Bergi
    Commented Feb 3, 2013 at 23:48
  • \$\begingroup\$ thanks... for some reason array and object were the same in my head \$\endgroup\$
    – user1732521
    Commented Feb 3, 2013 at 23:51
4
\$\begingroup\$

What I would probably do is each skill has a 'prereqs' array, which contains links to the prereq skill.

Then when you click the skill, you just loop through the prereqs and check that each one has skillpoint > 1

This would also satisfy the first skills that have no prereqs, they will just skip the loop because their prereq array would be empty.

This also allows if you ever need more than just 2 prereqs, since the array can have as many prereqs as you wan't and you won't have to add more 'if' checks

\$\endgroup\$
0
3
\$\begingroup\$

I'm not quite sure I've followed your logic, but it sounds like you have two pre-requisites (either of which might not be present), and you want to return false in any case where a prerequisite is present but is not met.

Can't you break out the testing of one prerequisite into its own function to simplify the logic? Something like this:

function checkOnePreReq(isRequired, value) {
    return !isRequired || value > 0;
}

function checkPreReq(e) {
    var $preReq = skill[$tab][e]['preReq'];
    var $value = skill[$tab][$preReq]['base'];

    var $preReq2 = skill[$tab][e]['preReq2'];
    var $value2 = skill[$tab][$preReq2]['base'];

    return checkOnePreReq($preReq, $value) &&
           checkOnePreReq($preReq2, $value2);
}
\$\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.