3
\$\begingroup\$

How could I improve this code to not use so many _.each calls and not always verify the object emptiness of its properties (or basically trying to make more readable) (I am mostly using Loadash)

  _.each(initial_obj, function(value, key){
    _.each(value, function(value2, key2){
      _.each(value2, function(value3, key3){
        _.isEmpty(new_obj[key3]) && (new_obj[key3] = {});
        _.isEmpty(new_obj[key3][key]) && (new_obj[key3][key] = []);
        new_obj[key3][key][key2] = value3;
      })
    })
  })

This code transforms this first object into the second:

obj = {'a': {
             '0': {'en': '1' ,   'es': '2',    'pt': '3'},
             '1': {'en': '11' ,  'es': '22',   'pt': '33'},
             '2': {'en': '111' , 'es': '222',  'pt': '333'},
            },
       'b': {
             '0': {'en': '4' ,   'es': '5',    'pt': '6'},
             '1': {'en': '44' ,  'es': '55',    'pt': '66'},
             '2': {'en': '444' , 'es': '555',  'pt': '666'},
             '3': {'en': '4444', 'es': '5555', 'pt': '6666'},
            },
       'c': {
             '0': {'en': '8' ,     'es': '9',    'pt': 'j'},
             '1': {'en': '88' ,    'es': '99',   'pt': 'jj'},
             '2': {'en': '888' ,   'es': '999',  'pt': 'jjj'},
             '3': {'en': '8888' ,  'es': '9999', 'pt': 'jjjj'},
             '4': {'en': '88888' , 'es': '99999','pt': 'jjjjj'},
            }
      }

and then my desired object would look like:

 new_obj= {'en': {
             'a': ['1', '11', '111' ],          
             'b': ['4', '44', '444', '4444'],
             'c': ['8', '88', '888', '8888', '88888']
            }}

....etc for the others

          'es':
             'a': []
             'b': []
             'c': []
          'pt':
             'a': []
             'b': []
             'c': []
          }

EDIT

At least I want a way to show that action in a more clear way

\$\endgroup\$
1
  • 2
    \$\begingroup\$ You can remove _.isEmpty call altogether: if (!new_obj[key3]) new_obj[key3] = {}; \$\endgroup\$
    – woxxom
    Commented Oct 26, 2017 at 23:55

2 Answers 2

2
\$\begingroup\$

Note: I assumed you use Lodash, though Underscore likely has the same or similar methods.

The primary reason the original code is hard to read is that the root keys are not known until the third loop. However, when you use the produced object, you know the root keys first.

With this in mind, I propose that you don't need to build new_obj at all. Instead, what you need is a function to produce new_obj.en when you pass en.

Thankfully, this is much simpler than your original code.

let obj = {
  a: {
   '0': {'en': '1' ,   'es': '2',    'pt': '3'},
   '1': {'en': '11' ,  'es': '22',   'pt': '33'},
   '2': {'en': '111' , 'es': '222',  'pt': '333'},
  },
 b: {
   '0': {'en': '4' ,   'es': '5',    'pt': '6'},
   '1': {'en': '44' ,  'es': '55',    'pt': '66'},
   '2': {'en': '444' , 'es': '555',  'pt': '666'},
   '3': {'en': '4444', 'es': '5555', 'pt': '6666'},
  },
 c: {
   '0': {'en': '8' ,     'es': '9',    'pt': 'j'},
   '1': {'en': '88' ,    'es': '99',   'pt': 'jj'},
   '2': {'en': '888' ,   'es': '999',  'pt': 'jjj'},
   '3': {'en': '8888' ,  'es': '9999', 'pt': 'jjjj'},
   '4': {'en': '88888' , 'es': '99999','pt': 'jjjjj'},
  }
}

const getTranslations = (obj, lang) => _.mapValues(obj, value => _.map(value, lang))

console.log('en', getTranslations(obj, 'en'))
console.log('es', getTranslations(obj, 'es'))
<script src="https://cdn.jsdelivr.net/lodash/4/lodash.min.js"></script>

If, you really need the full new_obj, you can simply build it using the above method for each language. To get the language keys, you can use one of the following snippets

// Assumes there is a 0 index in the first object of obj
// And that all languages are present in that object
_.keys(_.head(_.values(obj))[0])
// Slower, gets all the keys in all the objects and returns the unique array   
_.uniq(_.flattenDeep(_.values(obj).map(v => _.values(v).map(v => _.keys(v)))))
\$\endgroup\$
0
3
\$\begingroup\$

If you want not to have callback hell, my suggestion is to use classes. You can find a lot of techniques for how to define a class. I did it my way. You may change what you want. I know that many people would suggest some changes, but that will be about specific rules, not about the full decision. I suppose a majority would agree that my way has a right to live.

My problem while developing a class was that I did not know about the semantics of your data. It is a very good start to find out the semantics, split by items, give them names. Then you would get a decision from your logic. I gave some names to variables and methods but I suppose it likely should be changed.

function Transfer(source) {
    this.source = source;
    this.result = {};
}

Transfer.prototype = {
    constructor: Transfer,

    make: function() {
        var self = this;
        _.each(this.source, function (val, key) {
            self.processNames(val, key);
        });
        return this.result;
    },

    processNames: function(data, name) {
        var self = this;
        var path = [name];
        _.each(data, function (val, key) {
            self.processIndexes(val, path);
        });
    },

    processIndexes: function(data, path) {
        var self = this;
        _.each(data, function (val, key) {
            var newPath = (path).slice(0);
            newPath.unshift(key);
            self.processCodes(val, newPath);
        });
    },

    processCodes: function(data, path) {
        var current = this.result[path[0]] = this.result[path[0]] || {};
        current = current[path[1]] = current[path[1]] || [];
        current.push(data);
    }
};

var transfer = new Transfer(initial_obj);
transfer.make();

One more improvement might be to add a static method, e.g. .make:

Transfer.make = function (source) {
    var inst = new this(source);
    return inst.make();
};

I would not advise you to return any values from a constructor, as it is just for setting options. Generally, I would use controllers only for defining properties. Otherwise you need a stub prototype while testing which is not an elegant design.

I tested your code. It works as expected. I did not use indexes when setting values to array. I just used Array#push. If you have the structure that you provided, it should work.

I know that this technique has more lines of code than yours. But it has several advantages:

  • it is open for testing
  • it is open for extending
  • it is more human readable
  • it is more clear
\$\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.