H
H
hagen17782014-10-31 18:34:28
JavaScript
hagen1778, 2014-10-31 18:34:28

What could be wrong with writing a function?

I had a chance to solve a test task in JavaScript. The task was to implement the CSV parsing function:

var csv_by_name = compile_csv_search(
    "ip,name,desc\n"+
    "10.49.1.4,server1,Main Server\n"+
    "10.52.5.1,server2,Backup Server\n",
    "name");

console.log(csv_by_name("server2"));
console.log(csv_by_name("server9"));

..will print:
{ip: "10.52.5.1", name: "server2", desc: "Backup Server"}
undefined
(The return values ​​are objects, not strings.)
My solution to the problem looked like this:
function compile_csv_search(text, byField) {
        var text = text.split('\n'),
                headers = text[0].split(','),
                byIndex = 0;

        $.each(headers, function (index, field) {
            if (field == byField)
                byIndex = index;
        });
        text.shift();
        text.pop();

        return function (needle) {

            var curEl = '',
                    result = {};
            $.each(text, function (i, element) {
                curEl = element.split(',');
                if (curEl[byIndex] == needle) {
                    $.each(headers, function (index, header) {
                        result[ headers[index] ] = curEl[index];
                    });
                    return;
                }
            })
            return result;
        }
    }

The solution was regarded as not of sufficient quality. Please help me understand the reasons for such an assessment, since I could not get a response from the employer.

Answer the question

In order to leave comments, you need to log in

3 answer(s)
R
Rrooom, 2014-10-31
@hagen1778

- Why use jQuery?
- to add laziness, so that the function would only parse data on demand
- Poor knowledge of the language library
a. The first loop is replaced by indexOf
b. shift-pop... Ugly, there are slices.
- The most terrible in the most returned function! Why why!? Run in cycles every time? Come on in cycles, you don't even cache the result! Either completely parse the data into an object at the time of the first request, or gradually inflate it as you access the elements.

J
Jeiwan, 2014-10-31
@Jeiwan

My opinion: you are missing structure in your code. When calling compile_csv_search , we pass some data in which we will then need to search, so it would be logical to organize some kind of storage. When calling csv_by_name, we need to search through this storage. Here, I sketched my version on my knee:

function compile_csv_search(csv, field) {
  csv = csv.split("\n")
  var h = csv.shift().split(",");
  var res = [];

  for (var i = 0; i < csv.length; i++) {
    var obj = {};
    for (var j = 0; j < h.length; j++) {
      obj[h[j]] = csv[i].split(",")[j];
    }
    res.push(obj);
  }

  return function(id) {
    for (var i = 0; i < res.length; i++) {
      if (res[i][field] === id) {
        return res[i];
        break;
      }
    }
  };
};

Well, you need to check for the parameters passed to the function.
The fact that you use jquery for loops could scare off the employer - indeed, a strange decision.

P
personaljs, 2014-10-31
@personaljs

Noticeably poor knowledge of the language, as noted above. Just one more example:

var parserCSV = function (string, field){
  var parseString = string.split(/\n/),
    attrs = parseString.shift().split(',');

  var models = parseString.map(function (item) {
    var result = {};
    item.split(',').forEach(function (value, index){
      result[attrs[index]] = value;
    });
    return result;
  });

  return function (value) {
    return models.filter(function (model) {
      return model[field] === value;
    }).shift();
  };
}

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question