2
\$\begingroup\$

I submitted this exercise for a potential job. I really was hoping to get some meaningful feedback, so I can improve. Instead, they gave me a one line response: "showed a limited understanding of data management techniques and dom interaction." Not sure why they said dom interaction as this has nothing to do with the DOM. Is this really that bad, I know i repeated a bit of code (sorting by date), and my variable naming is not great. But what else? How would you have structured this differently? Would love to know your suggestions so I can improve and build and make the most of this.

Here are the Instructions:

Q: given an array of objects with the format of:

{

title: '',

body: '',

created: Date.toISOString(),

user: {

    name: '',

    id: #

}

}

with efficiency in mind, write a function that returns an array of objects in the format of:

{

name: '',

last_post_title: ''

}

containing the 30 most frequent posters of the last 3 months, sorted by the last time they posted.

Here is my code:

<!DOCTYPE html>
<html>
<head>
    <title>Code Exercise</title>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
</head>

<body>
    <script type="text/javascript">

        // using posts array as test array (only returning the top 2 posters since dataset is small)
        posts = [
        {
            title: 'Waiting 1000 Years for your SO',
            body: '',
            created: '2015-04-05',
            user: {
                    name: 'Rory',
                    id: 0
                }
        },
        {
            title: 'Better Regeneration',
            body: '',
            created: '2014-08-19',
            user: {
                    name: 'The Doctor',
                    id: 1
                }
        },
        {
            title: 'The Craziest Alien in All of Time!',
            body: '',
            created: '2014-08-09',
            user: {
                    name: 'The Doctor',
                    id: 2
                }
        },
                    {
            title: 'The Metaphysical aspects of the Tardis',
            body: '',
            created: '2014-08-27',
            user: {
                    name: 'The Doctor',
                    id: 3
                }
        },
                    {
            title: 'I am not so Impossible, Really',
            body: '',
            created: '2014-08-11',
            user: {
                    name: 'Clara',
                    id: 4
                }
        },
                    {
            title: 'The Bright Side of Being Shattered through Time',
            body: '',
            created: '2014-08-29',
            user: {
                    name: 'Clara',
                    id: 5
                }
        },           
                    {
            title: 'I Saw the Cracks in the Universe, and it was Awesome!',
            body: '',
            created: '2014-08-13',
            user: {
                    name: 'Amy',
                    id: 6
                }
        },
                                {
            title: 'My Imaginary Friend',
            body: '',
            created: '2014-07-15',
            user: {
                    name: 'Amy',
                    id: 7
                }
        }

        ];

        // this function is the answer to the project
        function projectAnswer() {

            // count number of posts, shove into counts array
            var counts = {};
            for (i=0; i < posts.length; i++)  {
                var a = posts[i].user.name;
                counts[a] = 1 + (counts[a] || 0); 
            }

            // massage counts to array of objects
            var newCounts = [];
            for (val in counts) {
                newCounts.push({
                    name: val,
                    numOfPosts: counts[val]    
                }); 
            }

            // sort by number of posts descending
            newCounts.sort(function (a, b) {
                if (a.numOfPosts > b.numOfPosts) {
                  return -1;
                }
                if (a.numOfPosts < b.numOfPosts) {
                  return 1;
                }
                // a must be equal to b
                return 0;
            });

            // Return top 30 posters (returning top 2 so it works with posts array)
            newCounts = newCounts.slice(0, 2);
            // console.log(newCounts);      

            // Use what we know now to pull the top two posters' post objects from posts array, shove into postsSorted array
            var postsSorted = [];
            function sortPosts(param) {
                for (i=0; i < param.length; i++)  {
                    param[i];
                    for (j=0; j < posts.length; j++) {
                        if (param[i].name == posts[j].user.name) {
                            postsSorted.push(posts[j]);
                        }
                    }
                }
            }
            sortPosts(newCounts);

            // sort by created date
            postsSorted.sort(function (a, b) {
                if (a.created > b.created) {
                  return -1;
                }
                if (a.created < b.created) {
                  return 1;
                }
                // a must be equal to b
                return 0;
            });


            function group(postsSorted){

                // get unique values
                var temp = {};
                var out = [];
                var final = [];
                for (var i = 0, l = postsSorted.length; i < l; i++) {
                  var id = postsSorted[i].user.name;
                    if (!temp[id]) {
                        temp[id] = [];
                        temp[id].push(postsSorted[i]);
                        }
                }
                for (var val in temp) {
                  out.push(temp[val]);
                }

                // Push into final array in format we want, array of objects with (name, last_post_title) 
                for (i=0; i < out.length; i++) {
                            final.push({
                                name: out[i][0].user.name,
                                last_post_title: out[i][0].title,
                                // this will list the date for verification purposes
                            });
                        }
                return final;
            }
            var postsSorted = group(postsSorted);

            document.write("<strong>Top 2 Posters' Last Post Sorted by Date: </strong>" +"<br/><br/>" + JSON.stringify(postsSorted));

        };
        projectAnswer();

        </script>

</body>

\$\endgroup\$
2
\$\begingroup\$

Without extensively combing through everything, a few things pop up to me:

  1. You use for...in loops in a few places. While they certainly have their place if used correctly, they often aren't, as they can easily produce unexpected results. A quick way to fix this would be to check if the property you're iterating over directly belongs to the object using hasOwnProperty.
  2. In several places, you use i or j as increment variables, but neither are defined anywhere in scope. i and j are global variables!
  3. document.write is seldom-seen because it rarely has a use anywhere in production. You may have more success inserting the result directly into an element in the DOM, which I suspect is where that comment came from.
  4. For loops have 3 parts: for(init, condition, expression) where expression is called every iteration. Every single iteration, a for loop will check if the condition is true, and if so, evaluate the expression and statement. All of your for loops check if somevar < something.length. Looking up the length value for an array takes time, so it is often a good idea to save this result somewhere and use it directly instead. Something like this would perform much faster on a large array:

var i, len;  //No global variables!

for(i = 0, len = something.length; i < len; ++i) {
    //calculate
}

I also pre-increment i rather than post-increment, as post-incrementing creates a copy of the variable, increments, then returns.

  1. The variable a doesn't mean anything to me, but it almost always points to a user.name.

Hopefully this helps!

\$\endgroup\$
1
\$\begingroup\$

Just as a suggestion, perhaps they were looking for a solution that utilized Array.prototype.map()?

function topPosters(count) {
    var users = [];

    posts.map(function(post){
        var userIndex = users.map(function(user) { return user.name; }).indexOf(post.user.name);

        if(userIndex === -1) {
            var user = {
                        name: post.user.name,
                        posts: 1,
                        lastPost: { 
                                    title: post.title,
                                    date: getDate(post.created)
                                  }
                        };

            users.push(user);
        } else {
            var user = users[userIndex];
            var postDate = getDate(post.created);

            if(postDate > user.lastPost.date) {
                user.lastPost = {
                    title: post.title,
                    date: postDate
                };
            }

            user.posts++;
        }
    });

    return users.sort(function(a,b){
        var x = a.posts;
        var y = b.posts;

        if(x < y) {
            return 1;
        }
        else if(x > y) {
            return -1;
        }
        else {
            if(a.lastPost.date < b.lastPost.date)
                return 1;
            else if(a.lastPost.date > b.lastPost.date)
                return -1;

            return 0;
        }
    }).map(function(user){
        return {
            name: user.name,
            last_post_title: user.lastPost.title
        };
    }).slice(0,count);
}

function getDate(dateStr) {
    var d1s = dateStr.split('-');
    return new Date(parseInt(d1s[0]),parseInt(d1s[1])-1,parseInt(d1s[2]));
}
\$\endgroup\$
  • \$\begingroup\$ Hi Tony- Interesting approach. Thanks for sharing. I was curious how ours compared efficiency wise, so I minified both our codes and took the average of 5 tests for load time. Yours loads in 1.98ms and mine in 2.25. So yours is roughly 12% more efficient. Assuming the dataset was 1000x bigger, your code would load .27 seconds faster. Seems like a wash, or are there situations where this could make a difference? \$\endgroup\$ – AnotherMike Aug 14 '15 at 1:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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