More efficient/clean piece of javascript code requested

Question

this.action = function(value){
        var deleted=false;
        for (var row in binaryObj) {
            if(value == 3){
                for(var i = 0 ; i < binaryObj[row].length; i++){
                    binaryObj[row].sort();
                    if(binaryObj[row][i]+binaryObj[row][i+1] === value) {                       
                        for(var j = 0; j < value; j++){
                            gameField.playField[row].pop(1);
                        }
                        deleted = true;
                        break;
                    }
                }
            }
            else{
                    for(var i = 0 ; i < binaryObj[row].length; i++){                
                            if(binaryObj[row][i] === value) {
                                for(var j = 0; j < value; j++){
                                    gameField.playField[row].pop(1);
                                }
                                deleted = true;
                                break;
                            }

                        if(deleted==true){
                            break;
                        }
                    }
                }
            if(deleted==true){
                break;
            }
        }
    };

As you can see, I am using a boolean to check whether an item has been deleted, so I can break out of all loops. Is there a better way to do this?

As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. — BCdotWEB
– BCdotWEB, Commented Feb 16, 2016 at 15:27
Why not just return from the function if there's nothing else you need to do? — user1149
– user1149, Commented Feb 16, 2016 at 17:43
Is this indentation weirdness a result of a copy-paste, or is this how your code actually looks? — Dan Oberlam
– Dan Oberlam, Commented Feb 16, 2016 at 18:34

Dan Oberlam · Accepted Answer · 2016-02-16 19:37:07Z

The first thing you need to do is fix your indentation - is it it is a little nonsensical.

Then, recognize that whenever you set deleted to true, you're done with the function. Instead of all of the extra stuff, just return from the function.

Also, in both branches you have the same behavior, where you seem to be clearing the gameField. You can consolidate that into a function.

this.action = function(value) {
    var deleted = false;
    for (var row in binaryObj) {
        for (var i = 0; i < binaryObj[row].length; ++i) {
            if (value == 3) {
                binaryObj[row].sort();
                cleanField(row, value);
                return;
            } else if (binaryObj[row][i] == value) {
                cleanField(row, value);
                return;
            }
        }
    }
}

function cleanField(row, limit) {
    for (var j = 0; j < limit; ++j) {
        gameField.playField[row].pop(1);
    }
}

Stack Exchange Network

More efficient/clean piece of javascript code requested

1 Answer 1

You must log in to answer this question.

Hot Network Questions

More efficient/clean piece of javascript code requested

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions