0
\$\begingroup\$

There is a function to convert object to the GET parameters for the ASP.Net controller. Is it possible to minimize the redundancy of the code, but not to the detriment of its readability?

function getUrlPairs(obj, prefix = "") {

    var getTypeOf = function (obj) {
        if (typeof obj == "undefined") {
            return "undefined";
        }
        if (obj === null) {
            return "null";
        }
        return Object.getPrototypeOf(obj).constructor.name;
    };

    var urlPairs = [],
    objType = getTypeOf(obj),
    propType;

    if (objType === "Object") {
        for (var propName in obj) {
            if (obj.hasOwnProperty(propName)) {
                propType = getTypeOf(obj[propName]);
                switch (propType) {
                case "String":
                    urlPairs.push(prefix + propName + "=" + encodeURIComponent(obj[propName]));
                    break;
                case "Number":
                case "Boolean":
                case "null":
                    urlPairs.push(prefix + propName + "=" + obj[propName]);
                    break;
                case "Date":
                    urlPairs.push(prefix + propName + "=" + encodeURIComponent(obj[propName].toJSON()));
                    break;
                case "Object":
                    urlPairs = urlPairs.concat(getUrlPairs(obj[propName], prefix + propName + "."));
                    break;
                case "Array":
                    urlPairs = urlPairs.concat(getUrlPairs(obj[propName], prefix + propName));
                    break;
                default:
                    break;
                }
            }
        }
    } else if (objType === "Array") {
        for (var i = 0; i < obj.length; i++) {
            propType = getTypeOf(obj[i]);
            switch (propType) {
            case "String":
                urlPairs.push(prefix + "[" + i + "]" + "=" + encodeURIComponent(obj[i]));
                break;
            case "Number":
            case "Boolean":
            case "null":
                urlPairs.push(prefix + "[" + i + "]=" + obj[i]);
                break;
            case "Date":
                urlPairs.push(prefix + "[" + i + "]=" + encodeURIComponent(obj[i].toJSON()));
                break;
            case "Object":
                urlPairs = urlPairs.concat(getUrlPairs(obj[i], prefix + "[" + i + "]" + "."));
                break;
            case "Array":
                urlPairs = urlPairs.concat(getUrlPairs(obj[i], prefix + "[" + i + "]"));
                break;
            default:
                break;
            }
        }
    } else {
        throw "support only arrays and objects";
    }

    return urlPairs;
}

var obj = {
    dirty: "&%[]?",
    nullable: null,
    numberProp: 123,
    now: new Date(),
    boolProp: true,
    obj: {
      objProp1: "objStr",
      objProp2: false,
    	nullable: null,
      objInObj: {
      	aa:"aa",
        bb: true
      }
    },
    arr: [{
        arrProp11: "str",
        arrProp12: 321,
        arrProp13: true,
        arrProp14: {
        	a: "absdefg",
          b: true
        }
      },
      {
        arrProp21: "rts",
        arrProp22: 987,
        arrProp23: false
      },
      ["arrInArr1", "arrInArr2"],
      [{test:"str"}],
      null,
    	new Date()
    ]
  },
  getTypeOf = function(obj) {
    if (typeof obj == "undefined") { return "undefined"; }
    if (obj === null) { return "null"; }
    return Object.getPrototypeOf(obj).constructor.name;
  },
  getUrlPairs = function(obj, prefix = "") {

    var urlPairs = [],
      	objType = getTypeOf(obj),
      	propType;

    if (objType === "Object") {
      for (var propName in obj) {
        if (obj.hasOwnProperty(propName)) {
          propType = getTypeOf(obj[propName]);
          switch (propType) {
            case "String":
              urlPairs.push(prefix + propName + "=" + encodeURIComponent(obj[propName]));
              break;
            case "Number":
            case "Boolean":
            case "null":
              urlPairs.push(prefix + propName + "=" + obj[propName]);
              break;
            case "Date":
              urlPairs.push(prefix + propName + "=" + encodeURIComponent(obj[propName].toJSON()));
              break;
            case "Object":
              urlPairs = urlPairs.concat(getUrlPairs(obj[propName], prefix + propName + "."));
              break;
            case "Array":
              urlPairs = urlPairs.concat(getUrlPairs(obj[propName], prefix + propName));
              break;
            default:
              break;
          }
        }
      }
    } else if (objType === "Array") {
      for (var i = 0; i < obj.length; i++) {
      		propType = getTypeOf(obj[i]);
          switch (propType) {
            case "String":
              urlPairs.push(prefix + "["+i+"]" + "=" + encodeURIComponent(obj[i]));
              break;
            case "Number":
            case "Boolean":
            case "null":
              urlPairs.push(prefix + "["+i+"]=" + obj[i]);
              break;
            case "Date":
              urlPairs.push(prefix + "["+i+"]=" + encodeURIComponent(obj[i].toJSON()));
              break;
            case "Object":
              urlPairs = urlPairs.concat(getUrlPairs(obj[i], prefix + "["+i+"]" + "."));
              break;
            case "Array":
              urlPairs = urlPairs.concat(getUrlPairs(obj[i], prefix + "["+i+"]"));
              break;
            default: break;
          }
      }
    }
    else { throw "support only arrays and objects"; }

    return urlPairs;
  };

var res = getUrlPairs(obj);
document.getElementById("output").innerHTML = res.join("\n") + "\n\n" + res.join("&");
body, textarea {
    margin: 0;
    overflow:hidden;
}
textarea {
    width: 100vw;
    height: 100vh;
    overflow:hidden;
    resize: none;
}
<textarea id="output"></textarea>

\$\endgroup\$
3
  • \$\begingroup\$ @chiril.sarajiu see obj in example above. \$\endgroup\$
    – XelaNimed
    Commented Feb 8, 2019 at 9:42
  • \$\begingroup\$ I would look in to JSON.stringify(), this could be written in a few lines of code with the JSON object. \$\endgroup\$
    – konijn
    Commented Feb 8, 2019 at 10:48
  • 1
    \$\begingroup\$ @konijn JSON.stringify is not suitable, because need a format that the ASP.Net controller would "understand". \$\endgroup\$
    – XelaNimed
    Commented Feb 8, 2019 at 10:56

2 Answers 2

1
\$\begingroup\$

General points

Not cyclic safe

The function is recursive so you have the danger of encountering cyclic references. If you are sure that they will not happen then you should be OK just don't call this function from another recursive function as the call stack is not infinite.

Avoid throwing

You are throwing an error at entry yet ignore the very same as you recurse. Why is the error not handled before you call the function. Having the error there means you need to wrap the call in a try catch. Are you unsure of the object you are passing, Where is it coming from and why would it not be as expected? Surely if its not as expected you would expect an empty array returned, rather than having to deal with an error.

If you must throw dont throw strings. See points below.

Be efficient

It cost money to run code so always be efficient

It is more efficient to close over a recursive function with objects that are shared. You create an new array urlPairs each time you recure. Then on returning you create yet another array when you concat the result. You can eliminate that overhead using closure.

Each time you you recurse you call getTypeOf on the object you pass. But you already know the type or you would not be recursing. Pass the object type when known, it will make things a little simpler.

Your code

Every line of code counts as a locations for a bug. Reducing the number of lines of codes is the best way to reduce the number of bugs.

  • Use function declarations function name() {... or arrow function const name = () => {... in favour over function expressions var name = function(){.... If you do use an expression then use a constant.
  • Use for of rather than for in
  • Using for of and Object.entries will get you the property name and value. for(const [name, value] of Object.entries(obj)) { .
  • NEVER throw strings. Always throw error like object and have a preference for already defined error types. eg ...} else { throw new RangeError("support only arrays and objects") ...

  • Use === and !== rather than == and !=

  • Reduce noise by keeping it as simple as possible.

    • The line if (typeof obj == "undefined") { return "undefined"; } can be simplified to if (ob === undefined) { return "undefined" }.
    • On the same point you return the string "undefined" yet never test for it, it could be almost any value. So you can just as well return undefined as follows if (ob === undefined) { return }
    • return Object.getPrototypeOf(obj).constructor.name; can be return obj.constructor.name.
    • Array.concat(array) is slow and a memory and GC thrasher. Use Array.push(...array) Thus urlPairs = urlPairs.concat(getUrlPairs(obj[propName], prefix + propName)), becomes urlPairs.push(...getUrlPairs(obj[propName], prefix + propName)),,
    • You add prefix to index 10 times, code should never have the same operation repeated within one function so many times.
    • If you don't do anything in the default clause of a switch then don't add it, it's just needless noise.
  • You have repeated the same code for "Objects" and "Array" items, the only difference is that the array index is wrapped in [] and the object key is not. You can create a function to wrap the key depending on the object type.

  • You can use bracket notation to replace long switch case lists. (See example)

Example

The example ignores the throw and iterate recursively via an internal function. If the start object is not the correct type it just returns an empty array.

The different object types are handle via an object containing named calls typeFunc. Eg typeFunc.Date will handle Date type objects.

The iteration start via typeFunc.Object or typeFunc.Array These functions also pass on how to deal with the object path via iterate function wrap argument.

Everything is pushed onto the one pairs array that is return when complete.

The function IS NOT cyclic safe.

function getUrlPairs(obj) {
  const getType = obj => obj === undefined ? "" : (obj === null ? "null" : obj.constructor.name);
  const encode = str => encodeURIComponent(str);
  const simple = (path, val) => path + "=" + encode(val);
  const typeFunc = {
    Number: simple, Boolean: simple, null: simple, String: simple,
    Date: (path, val) => path + "=" + encode(val.toJSON()),
    Object: (path, val, dot = ".") => iterate(val, path + dot, name => name),
    Array: (path, val) => iterate(val, path, name => `[${name}]`),
  }
  const pairs = [], type = getType(obj);
  function iterate (obj, prefix, wrap) {
    for (const [name, val] of Object.entries(obj)) {
      const call = typeFunc[getType(val)];
      call && pairs.push(call(prefix + wrap(name), val));
    }
  }
  typeFunc[type] && typeFunc[type]("", obj, "");
  return pairs;
}

Running example

function getUrlPairs(obj) {
  const getType = obj => obj === undefined ? "" : (obj === null ? "null" : obj.constructor.name);
  const encode = str => encodeURIComponent(str);
  const simple = (path, val) => path + "=" + encode(val);
  const typeFunc = {
    Number: simple, Boolean: simple, null: simple, String: simple,
    Date: (path, val) => path + "=" + encode(val.toJSON()),
    Object: (path, val, dot = ".") => iterate(val, path + dot, name => name),
    Array: (path, val) => iterate(val, path, name => `[${name}]`),
  }
  const pairs = [], type = getType(obj);
  function iterate (obj, prefix, wrap) {
    for (const [name, val] of Object.entries(obj)) {
      const call = typeFunc[getType(val)];
      call && pairs.push(call(prefix + wrap(name), val));
    }
  }
  typeFunc[type] && typeFunc[type]("", obj, "");
  return pairs;
}


getUrlPairs({
    dirty: "&%[]?",
    nullable: null,
    numberProp: 123,
    now: new Date(),
    boolProp: true,
    obj: {
      objProp1: "objStr",
      objProp2: false,
    	nullable: null,
      objInObj: {
      	aa:"aa",
        bb: true
      }
    },
    arr: [{
        arrProp11: "str",
        arrProp12: 321,
        arrProp13: true,
        arrProp14: {
        	a: "absdefg",
          b: true
        }
      },
      {
        arrProp21: "rts",
        arrProp22: 987,
        arrProp23: false
      },
      ["arrInArr1", "arrInArr2"],
      [{test:"str"}],
      null,
    	new Date()
    ]
}).forEach(str => 
    show.appendChild(Object.assign(document.createElement("div"),{textContent: str}))
)
<code id="show"></code>

\$\endgroup\$
2
  • \$\begingroup\$ It is necessary to fix the line call && pairs.push(call(prefix + wrap(name), val)); in iterate function, since there is no check for undefined. Otherwise, the resulting array will contain undefined elements. \$\endgroup\$
    – XelaNimed
    Commented Feb 13, 2019 at 8:52
  • \$\begingroup\$ @XelaNimed There is no function for "undefined" (empty "" string returned by getType for var that is undefined) in typeFunc so how can you get undefined items? The type must be defined by name in the obj typeFunc to make it into the result. \$\endgroup\$
    – Blindman67
    Commented Feb 13, 2019 at 10:38
1
\$\begingroup\$

First, I would step away from the code and look for tools in the ecosystem that already deals with this or other viable approaches. You could:

  • Just send JSON either by GET queries or POST request bodies. Surely ASP.NET has libraries to deserialize JSON into native objects.
  • Spring (Java) is able to map query parameters and request bodies into Java objects (@RequestBody, @RequestParam, and friends). Check if ASP.NET has similar libraries or APIs that allow you to do this.
  • If you really have no choice but to use GET params, check out jQuery.param(). Not sure if it's the format ASP.NET uses, but works great in PHP.
\$\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.