4
\$\begingroup\$

I have implemented a recursive function that converts an object's keys according to another lookup table/map. You are able to convert back and forth using the 3rd swap_conversion_table_key_value boolean argument.

My use case is to convert object keys to single characters to slim down the amount of characters generated from JSON.stringify. Then be able to convert it back to full keys on another client.

Along with the usual code review criteria(mainly clarity), I am wondering if I just reinvented the wheel or overthought the whole problem. The code does seem a bit long for the functionality and I am not keen how it directly modifies the object(wish it would return a new object). This means that you have to clone the data before every use to keep all the references to the original data happy.

Demo: jsFiddle

Usage:

recursiveConvertKeys(resultant_data, conversion_table, false);

// 3rd parameter defines whether we should swap the key-value in the table/map (good for converting back to the original data)
recursiveConvertKeys(resultant_data, conversion_table, true);

Code:

Code available as a GitHub Gist

function recursiveConvertKeys(data_object, conversion_table, swap_conversion_table_key_value, __is_recursive_iteration, __current_object_level, __current_conversion_table_level)
{
    // Do not pass in parameters for the double underscore arguments. These are private and only used for self recursive calling
    //
    // Demo: http://jsfiddle.net/MadLittleMods/g3g0g1L4/
    // GitHub Gist: https://gist.github.com/MadLittleMods/7b9ec36879fd24938ad2
    // Code Review: http://codereview.stackexchange.com/q/69651/40165
    //
    /* Usage: 
        var data = {asdf: 1, qwer: 2};
        var conversion_table = {asdf: 'a', qwer: 'q'};

        // Clone the data so we don't overwrite it
        var resultant_data = $.extend(true, {}, data);
        // Now execute the key converting process
        recursiveConvertKeys(resultant_data, conversion_table, false);
        console.log("Reversed Data:", resultant_data);

        // If you want to reverse the process simply pass true for the `swap_conversion_table_key_value` argument
        recursiveConvertKeys(resultant_data, conversion_table, true);
        console.log("Back to normal Data:", resultant_data);
    */


    // Start at the root of the objects when we invoke this method
    __current_object_level = __is_recursive_iteration ? __current_object_level : data_object;
    __current_conversion_table_level = __is_recursive_iteration ? __current_conversion_table_level : conversion_table;


    if(typeof __current_object_level == "object")
    {
        // Make the iterate object
        var iterate_object = Object.keys(__current_object_level);
        //console.log('iter', iterate_object);

        iterate_object.map(function(key, index, array) {
            // Check to make sure this is part of the object itself
            if (__current_object_level.hasOwnProperty(key))
            {
                if(__current_conversion_table_level)
                {
                    var new_key = null;
                    if(!swap_conversion_table_key_value)
                    {
                        if(typeof __current_conversion_table_level[key] == "object")
                            new_key = __current_conversion_table_level[key]['_short'];
                        else
                            new_key = __current_conversion_table_level[key];
                    }
                    else
                    {
                        // We have to search through all of the current level to match the value to curernt object key since we swapped
                        var table_level_keys = Object.keys(__current_conversion_table_level);
                        for(var i = 0; i < table_level_keys.length; i++)
                        {
                            var curr_level_table_key = table_level_keys[i];

                            var key_to_compare = null;
                            var curr_level_table_value = __current_conversion_table_level[curr_level_table_key];
                            if(typeof curr_level_table_value == "object")
                                key_to_compare = curr_level_table_value['_short'];
                            else
                                key_to_compare = curr_level_table_value;

                            // If it is a match, we found it :)
                            if(key_to_compare == key)
                            {
                                // Now use the key from the conversion table instead of the value
                                new_key = curr_level_table_key;

                                // Break out of the for loop after we found it
                                break;
                            }
                        }
                    }

                    // If there is actually a new key, replace it in our object
                    if(new_key)
                    {
                        renameProperty(__current_object_level, key, new_key);
                    }

                    //console.log('key', key, new_key);

                    // Only keep going if there actually was a new_key
                    // Or there is a array to look through the items on
                    var is_current_key_array_index = key%1 == 0; // If the current key is a positive integer, we assume it is an array key
                    if(new_key || is_current_key_array_index)
                    {
                        // Use the new key if it was available
                        // Because that is what the object property is changed to from above
                        var value = __current_object_level[new_key ? new_key : key];

                        //console.log('current', value, __current_conversion_level);

                        // If we are swapping then the `key` will not be found in the table as it is ass-backwards.
                        var table_key = swap_conversion_table_key_value ? new_key : key;

                        // If the current key is a array, maintain the `_array_item` conversion level we set the level prior
                        // Otherwise continue down the tree
                        var next_conversion_level = is_current_key_array_index ? __current_conversion_table_level : __current_conversion_table_level[table_key];
                        if(typeof __current_conversion_table_level[table_key] == "object")
                        {
                            // If the current value is an array set up the conversion level for the items
                            if(Object.prototype.toString.call(value) === '[object Array]')
                            {
                                next_conversion_level = __current_conversion_table_level[table_key]['_array_item'];
                            }
                            else
                            {
                                next_conversion_level = __current_conversion_table_level[table_key]['_object'];
                            }
                        }
                        //console.log('next', next_conversion_level);
                        recursiveConvertKeys(data_object, conversion_table, swap_conversion_table_key_value, true, value, next_conversion_level);
                    }
                }
            }
        });
    }
}

The format for the conversion table is below. I am not set on this format so feel free to suggest something better for the table/map.

var conversion_table = {
    "psdf": "p", 
    "qwer": "q",
    "dict": {
        "_short": "d",

        "_object": {
            "one": "o",
            "two": "t",
            "three": "r"
        }
    },
    "candidates": {
        "_short": "c",

        "_array_item": {
            "ip": "i",
            "port": "p"
        }
    }
}

And some accompanying test data:

var test_data = {
    "psdf": "pcodereview",
    "qwer": "qcodereview",
    "dict": {
        "one": "1",
        "two": "2",
        "three": "3"
    },
    "candidates": [
        {
            "ip": "0.0.0.0",
            "port": 65000
        },
        {
            "ip": "127.0.0.1",
            "port": 65000
        }
    ]
}
\$\endgroup\$

2 Answers 2

Reset to default
3
\$\begingroup\$

Great question,

Can you take the truth?
The real answer is that this is overkill. Send your JSON to other machines/clients with HTTP compression (gzip) and the benefits of slimming down are close to non-existing.

Divide and conquer
Still, this is a fun question. After looking at this for a while, I've come to the conclusion that you have so much code because you tried too hard to apply DRY. Converting keys and reverting keys are different enough to deserve separate functions. They will look tantalizingly similar, but I am quite sure that merging them is wrong.

Naming convention
Furthermore, stop using _ and __ as prefixes to variables, it is not idiomatic for JavaScript. And start using lowerCamelCase, so __is_recursive_iteration should be isRecursiveIteration.

Name that thing
Furthermore, to quote Humpty Dumpty

"When I use a word," Humpty Dumpty said, in rather a scornful tone, "it means just what I choose it to mean—neither more nor less." "The question is," said Alice, "whether you can make words mean so many different things." "The question is," said Humpty Dumpty, "which is to be master—that's all.” ― Lewis Carroll

When you declare curr_level_table_key it is the only 'key' in the scope, feel free to declare it simply as key, this would make your code far easier to understand. If you are not comfortable because it does not convey enough info, you could comment it (I would not do this personally):

var key = table_level_keys[i]; //Current level table key <- That does not really make sense to me

Commenting
Also your commenting is too excessive for what really should be a simple, recursive, key swapping algorithm. At least convert your 2 line comments to 1 line comments, remove obvious comments, and all remaining multi line comments should go in front of the function.

Worst offender:

 
// Break out of the for loop after we found it
break;

Avoid the arrow pattern
You know that things are getting too complex when you see the arrow pattern, like here:

                        //console.log('next', next_conversion_level);
                        recursiveConvertKeys(data_object, conversion_table, swap_conversion_table_key_value, true, value, next_conversion_level);
                    }
                }
            }
        });
    }
}

In this particular case you can do this by using the continue statement if you know that nothing else must be done in the loop, so consider using

if (!object.hasOwnProperty(key)){
  continue;
}

Housekeeping
Remove commented out code, your code is already hard to read and follow.

Counter proposal

// from: http://stackoverflow.com/a/4648411/796832
// Check for the old property name to avoid a ReferenceError in strict mode.
function renameProperty(object, oldName, newName) {
  if (object.hasOwnProperty(oldName)) {
    object[newName] = object[oldName];
    delete object[oldName];
    return object[newName];
  }
}

function convertKeys(object, map) {
  if (typeof object != "object") {
    return;
  }
  //Iterate over the object
  Object.keys(object).map(function(key) {
    var mappedKey = map[key];
    if (!mappedKey) {
      return;
    }
    var value = object[key];
    if (mappedKey instanceof Object) {
      if (mappedKey._short) {
        value = renameProperty(object, key, mappedKey._short);
      }
      if (value instanceof Array) {
        for (var i = 0, length = value.length; i < length; i++) {
          convertKeys(value[i], mappedKey._array_item);
        }
      } else if (value instanceof Object) {
        convertKeys(value, mappedKey._object);
      }
    } else {
      renameProperty(object, key, mappedKey);
    }
  });
}

function findValueKey(object, searchValue) {
  var keys = Object.keys(object), key, value;
  
  for (var i = 0, length = keys.length; i < length; i++) {
    key = keys[i];
    value = object[key];   
    if (value === searchValue) {
      return key;
    }
    if( value instanceof Object && value._short == searchValue ){
      return key;
    }
  }
}

function revertKeys(object, map) {
  if (typeof object != "object") {
    return;
  }
  //Iterate over the object
  Object.keys(object).forEach(function(key) {
    var mappedKey = findValueKey(map, key);
    if (!mappedKey) {
      return;
    }
    var value = renameProperty(object, key, mappedKey),
        subMap = map[mappedKey];
    if (subMap instanceof Object) {
      if (value instanceof Array) {
        for (var i = 0, length = value.length; i < length; i++) {
          revertKeys(value[i], subMap._array_item);
        }
      } else if (value instanceof Object) {
        revertKeys(value, subMap._object);
      }
    }
  });
}




$('textarea').tabOverride();

$('.go').on('click', function() {
  // Tests
  // ---------------------------
  var data = JSON.parse($('.data-box').val());
  var conversion_table = JSON.parse($('.conversion-table-box').val());

  // Clone the data so we don't overwrite it
  var resultant_data = $.extend(true, {}, data);
  // Now execute the key converting process
  convertKeys(resultant_data, conversion_table);
  $('.result').html(JSON.stringify(resultant_data, null, '\t'));
  console.log("Reversed Data:", resultant_data);

  // If you want to reverse the process simply pass true for the `swap_conversion_table_key_value` argument
  revertKeys(resultant_data, conversion_table);
  $('.result-back-to-normal').html(JSON.stringify(resultant_data, null, '\t'));
  console.log("Back to normal Data:", resultant_data);

}).trigger('click');
*, *:before, *:after {
    -moz-box-sizing: border-box;
    -webkit-box-sizing: border-box;
    box-sizing: border-box;
}

html, body
{
    width: 100%;
    height: 100%;
    margin: 0;
    padding: 0;
    
    font-family: Arial, sans-serif;
}

pre, textarea 
{
    tab-size: 4;
    -moz-tab-size: 4;
    -o-tab-size: 4;
    -webkit-tab-size: 4;
}

textarea, pre
{
    width: 100%;
    min-height: 150px;
    
    padding: 4px;
}

.side-by-side-section
{
    display: flex;
}

.side-by-side-section > *
{
    flex: 1;
}


.code-block
{
    background: rgba(0, 0, 0, 0.1);
    border: 1px solid rgba(0, 0, 0, 0.2);
}
<!DOCTYPE html>
<html>
<head>
<meta name="description" content="Map keys" />
<script src="//code.jquery.com/jquery-2.1.1.min.js"></script>
<script src="//rawgit.com/wjbryant/taboverride/master/build/output/taboverride.js"></script>
<script src="//rawgit.com/wjbryant/jquery.taboverride/master/src/jquery.taboverride.js"></script>
  <meta charset="utf-8">
  <title>JS Bin</title>
</head>
<body>
<button class="go">Go</button>
Write data and conversion table in JSON.
<br />
<div class="side-by-side-section">
    <div>
        <div>Data:</div>
        <textarea class="data-box">
{
	"psdf": "pcodereview",
	"qwer": "qcodereview",
	"dict": {
		"one": "1",
		"two": "2",
		"three": "3"
	},
	"candidates": [
		{
			"ip": "0.0.0.0",
			"port": 65000
		},
		{
			"ip": "127.0.0.1",
			"port": 65000
		}
	]
}    
        </textarea>
    </div>
    <div>
        <div>Conversion table:</div>
        <textarea class="conversion-table-box">
{
	"psdf": "p", 
	"qwer": "q",
	"dict": {
		"_short": "d",
            
		"_object": {
			"one": "o",
			"two": "t",
			"three": "r"
		}
	},
	"candidates": {
		"_short": "c",

		"_array_item": {
			"ip": "i",
			"port": "p"
		}
	}
}
        </textarea>
    </div>
</div>
<div class="side-by-side-section">
    <div>
        <div>Converted:</div>
        <pre class="code-block"><code class="result"></code></pre>
    </div>
    <div>
        <div>Converted back:</div>
        <pre class="code-block"><code class="result-back-to-normal"></code></pre>
    </div>
</div>
</body>
</html>

\$\endgroup\$
3
  • \$\begingroup\$ Thank you konijn, I am still reading through the answer! The reason I am slimming down the keys is not to reduce storage or bandwidth usage. It is because I am using this stringified data(along with some other compression stuff) as a copy/paste-able blob/token. The smaller it is, the easier it is to select and transfer whether it be email, IM, etc. TLDR; I am worried about the number of characters, not actual number of bytes it takes up. \$\endgroup\$ Commented Nov 18, 2014 at 19:38
  • \$\begingroup\$ I made a new answer that combines convertKeys and revertKeys in a simple way. I highly appreciate the review as I would never have ended up at my code without your answer. \$\endgroup\$ Commented Nov 19, 2014 at 16:02
  • \$\begingroup\$ Nice, I hope you will still mark my answer as correct :P \$\endgroup\$ Commented Nov 19, 2014 at 18:58
3
\$\begingroup\$

I was able to condense the convertKeys and revertKeys into one function but still maintain a lean, understandable code base.

Please check out konijn's answer because it outlines what I am going to say and it really opened my eyes on new ways to code this function, especially flow. I thought the code below was different enough to deserve its own answer, even though many of the points are already iterated in konijn's answer.

Commenting

I had way too many comments and once you have a short function, a full example in a multi-line comment just is not necessary anymore.

Naming

I switched back to using camelCase. I have no idea why I used underscore separated variables but I assume it is a side effect of getting some pretty long and hairy variable names.

Flow

Along with the code being much nicer to read, I am now returning a new cloned object with the keys converted. This does have a side effect of now relying on jQuery.extend or some other extend function to clone the object, but it produces a better way to work with the function.

var resultantData = convertKeys(data, conversionTable);

// 3rd parameter defines whether we should swap the key-value in the table/map (good for converting back to the original data)
var backToNormalData = convertKeys(resultantData, conversionTable, true);

Full Code:

Demo: jsFiddle

Updated GitHub Gist

// These are used to determine the map/table format
var const_objectKeyPropertyName = '_short';
var const_objectValuePropertyName = '_object';
var const_arrayKeyPropertyName = '_array_item';

function convertKeys(object, map, reverseMap)
{
    if (typeof object != "object" || map == null) {
        return object;
    }

    // Clone the object so we don't modify the original
    var resultantObject = $.extend(true, {}, object);

    // Iterate over the object
    Object.keys(resultantObject).forEach(function(key) {
        var mapKey = reverseMap ? findKeyFromValue(map, key) : key;

        var newKeyName = reverseMap ? mapKey : map[mapKey];
        if(typeof newKeyName == "object") {
            newKeyName = newKeyName[const_objectKeyPropertyName];
        }

        if(!newKeyName) {
            return resultantObject;
        }

        renameProperty(resultantObject, key, newKeyName);


        var value = resultantObject[newKeyName];

        if (value instanceof Array) {
            for (var i = 0, length = value.length; i < length; i++) {
                resultantObject[newKeyName][i] = convertKeys(value[i], map[mapKey][const_arrayKeyPropertyName], reverseMap);
            }
        }
        else if(typeof value == "object") {
            resultantObject[newKeyName] = convertKeys(value, map[mapKey][const_objectValuePropertyName], reverseMap);
        }

    });

    return resultantObject;
}

function renameProperty(object, oldName, newName)
{
    // from: http://stackoverflow.com/a/4648411/796832

    // Check for the old property name to avoid a ReferenceError in strict mode.
    if (object.hasOwnProperty(oldName)) {
        object[newName] = object[oldName];
        delete object[oldName];
    }
    return object;
}

function findKeyFromValue(object, searchValue)
{
    var keys = Object.keys(object);
    var resultantKey = false;
    for(var i = 0; i < keys.length; i++) {
        var key = keys[i];
        var value = object[key];
        if(typeof value == "object") {
            value = value[const_objectKeyPropertyName];
        }

        if(value == searchValue) {
            resultantKey = key;
            break;
        }
    }

    return resultantKey;
}
\$\endgroup\$

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.