current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 0 down vote favorite

I have to parse a json object that can be a string, array, array of strings and array of object. I realised that it's not good from the beginning that one object can be many types, but I can't change the code from upstream so I'll have to deal it in my code instead. I'm building a pixel library for modern browser so I'm not using jQuery or lodash. I'm supporting most modern browser and IE >= 9

Here's the example of the data that can be returned

"author": {
 "@type": "Person",
 "name": "Author"
}

Or

"author":[{"@type":"Person","name":"Author"}]

Or

"author": 'Author'

Or

"author": ['Author', 'Author1']

And this is my code. 

  let obj = {};
  try {
    const json = document.querySelector('div.json');
    if (json) {
      let disc = JSON.parse(json.innerHTML);
      let authors = disc.author;

      if (typeof authors !== 'undefined' && Array.isArray(authors) && authors.length > 0) {
        authors = authors.map((author) => {
          if (typeof author === 'object' && author.name) {
            return author.name;
          } else {
            return author;
          }
        });
      }

      if (typeof authors !== 'undefined' && !Array.isArray(authors) && typeof authors === 'object') {
        authors = [authors.name];
      }

      if (typeof authors !== 'undefined' && typeof authors === 'string') {
        authors = authors.split(',');
      }

      obj.cAu: authors.join(',');
    }
  } catch (e) { }

  return obj;

My question is, is there a better way to do this in a more efficient way?

share|improve this question
    
Could you include the content "div.json" has in your html. We need to have your sample json to be able to review your code. – Tolani Jaiye-Tikolo Jul 22 at 23:27
up vote 2 down vote accepted

Personally I would have written the function as below. One thing that stood out that I noticed is that you are not returning anything in the catch statement of your try. So there is a certain 'ambiguity' as to what the function will return.

Here is my reasoning for the code style below:

  1. I expect the invoker to provide me with the JSON string that needs to data extracted from it, so that I can perform some immediate input validation. If the input is falsey I will return an empty string.

  2. I check whether the parsed result is an instance of an array (undefined or null will not pass this check)

  3. If it is an instance of an array I will check whether it is an array of objects, otherwise I will assume it is a string. I map these entries, and join them with a comma and return the result

  4. if the type is of an object (and not array since we already checked for that), I will return the name property of that object.

Feel free to wrap this in a try'catch but make sure you return something on the catch. I assume the invoker is expecting a string back.

function parseAuthor(authorString){
  if(!authorString){
    return '';
  }

  const authorObj = JSON.parse(authorString);

  if (typeof authorObj === 'string') {
    return authorObj;
  }

  if (authorObj instanceof Array) {
    entries = authorObj.map((a) => {
      if(typeof a === 'object' && a.name){
        return a.name;
      } else {
        return a;
      }
    });

    return entries.join(',');
  }

  if (typeof authorObj === 'object' && authorObj.name) {
    return authorObj.name;
  }

  return '';
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.