2
\$\begingroup\$

I'm unsure about how to simplify this method properly. The method is basically responsible to allocate data.

The response object is a collection of objects. Each object contains an attribute called order and i have to distinguish between visible and invisible elements. Visible elements are represented by positive integers, invisible elements by a negative integers.

Also it is possible to overwrite the default state if the URL contains an parameter (hidden) with a list of hidden element ids. ?hidden=23,432,3,123

_hasParam() returns boolean

_getParam() returns string (separated by comma [23,432,3,123])

contains() returns boolean

prepare: function (response) {

    var visible = [];
    var invisible = [];

    if (_hasParam('hidden')) {

        var hidden = _getParam('hidden').split(',');

        angular.forEach(response, function (item) {
            if (hidden.contains(item.id)) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        });

    } else {

        angular.forEach(response, function (item) {
            if (item.order === -1) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        });

    }

    $rootScope.visible = visible;
    $rootScope.invisible = invisible;
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The only thing that stands out a bit is the fact that you do 2 very similar angular.forEach() calls. You could extract these functions out and do only 1 call. I would also make -1 a named constant. I would also declare all variables on top.

What you could consider, but I don't advise it, since it reeks too much of CodeGolf, is to simplify the 2 if statements to 1 single push() call

(hidden.contains(item.id)? invisible : visible).push( item );

This piece of code extracts the filter functions and applies my other suggestions:

prepare: function (response) {

    var visible = [],
        invisible = [],
        HIDE = -1,
        hidden, filter;

    if (_hasParam('hidden')) {
        hidden = _getParam('hidden').split(',');
        /* Hide based on the hidden parameter */
        filter = function (item) {
            if (hidden.contains(item.id)) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        }
    } else {
        filter = function (item) {
        /* Hide based on the order of the item */
            if (item.order === HIDE) {
                invisible.push(item);
            } else {
                visible.push(item);
            }
        }    
    }

    angular.forEach(response, filter);
    $rootScope.visible = visible;
    $rootScope.invisible = invisible;
}
\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.