6
\$\begingroup\$

I was given a practical task as part of an interview for a front-end developer position. There is a follow up interview and I'm looking for some constructive criticism / tough love to help me prepare.

The guidelines were:

  • Should work on IE8
  • Use CDN's for external libraries
  • Use whichever library(ies) you prefer
  • Should take 2 hours

The code below is condensed into one file for convenience. There is a working version at http://54.201.30.172/frontend-challenge/

<div id="app">
  <!-- Generated Secret Santa assignments should go here -->
  <div id='participant-list'>

  </div>
</div>


<script type="text/template" id="rootTemplate">
    <thead>
    <tr>
      <th></th>
    </tr>
  </thead>

  <tbody>
  </tbody>

  <tfoot>
    <tr>
      <td>
        <input type="checkbox" class="preventDups" name="preventDups" <% if(preventDuplicates){ %> checked <% } %> />No Buy Backs
      </td>
      <td></td>
      <td></td>


      <td></td>
      <td>
        <button class="shuffleButton">Reshuffle</button>
      </td>
    </tr>
  </tfoot>

</script>

<script type="text/template" id="oneParticipantTpl">

  <td> <%= name.first %> <%= name.last %> </td>
  <td>&nbsp;</td>
  <td> buys for </td>
  <td>&nbsp;</td>
  <% if(buysFor){ %>
    <td id="<%= buysFor.guid %>"> <%= buysFor.name.first %> <%= buysFor.name.last %> </td>
  <% } %>

</script>


<script type="text/javascript">


  (function(){  

    var UserModel = Backbone.Model.extend({

      idAttribute : 'guid',

      defaults:{
        "name": {
          "first": "",
          "last": ""
        },
        "email": "",
        "phone": "",
        "buysFor": null
      },

      initialize: function(){

        // The user cannot buy for themselves
        // Store their own id in an array of ids they can't buy for
        // Any additional users that they can't buy for will be added here too
        this.set('cantBuyFor', [this.get('guid')]);

        // Set the display name for convenience
        this.set('displayName', this.get('name').first + ' ' + this.get('name').last)
      }

    });

    var UsersCollection = Backbone.Collection.extend({

      model : UserModel,
      url : 'users.json',

      // Parse the users property from the ajax request
      parse: function(resp){
        return resp.users;
      }

    });

    var OneParticipantView = Mn.ItemView.extend({

      // Childview of All particiapnts view
      // Renders each row in the table
      template: '#oneParticipantTpl',
        tagName: 'tr'

    });

    var AllParticipantsViewModel = Backbone.Model.extend({

      // model to render and control whether recipients can buy for their buyers (buy backs)
      defaults : {
        preventDuplicates: true
      }
    });

    var AllParticipantsView = Mn.CompositeView.extend({

      template : '#rootTemplate',
      childView : OneParticipantView, 
      childViewContainer: 'tbody',
      tagName: 'table',
      model : new AllParticipantsViewModel(),

      // Make sure that we render the table when the collection is updated from the server
      collectionEvents: {
          'sync': 'reShuffle'
      },

      ui: {
        shuffleButton: '.shuffleButton',
        dupsCheckbox : '.preventDups'
      },

      events: {
        'click @ui.shuffleButton': 'reShuffle',
        'click @ui.dupsCheckbox' : 'changeDups'
      },

      changeDups: function(e){

        // toggle whether buy backs are enabled
        // Set the view-model property to whatever the user has selected and rerender the list
        e.preventDefault();
        var $checkbox = $(e.currentTarget);

        if($checkbox.is(':checked')){
          this.model.set('preventDuplicates', true);
        }else{
          this.model.set('preventDuplicates', false);
        }

        return this.render();
      },

      initialize : function(){
        // This can be used if the models are bootstrapped into the page rather than loaded over ajax
        //this.shuffle();
      },

      validRecipients: function(buyer, boughtFor){

        // Returns the list of people the buyer can buy for
        // This should be everyone less people who have already been bought for and anyone the buyer is forbidden to buy for (themselves)
        var validRecipients = this.collection.clone();
        validRecipients.remove(buyer.get('cantBuyFor'));
        validRecipients.remove(boughtFor);
        return validRecipients;
      },

      validSwaps: function(buyer){

        // Returns a list of people the buyer can swap without breaking rules
        var validSwaps = this.collection.clone();
        validSwaps.filter(function(recipient){
          return buyer.get('cantBuyFor').indexOf(recipient.get('guid')) === -1;
        });

        return validSwaps;
      },


      randomDraw: function(max){
        // Random number generator
        return Math.floor(Math.random() * max);
      },

      preventDuplicatesDraw: function(buyer, recipient){

        // Prevent the recipient from buying back to the buyer if duplicates are disabled
        if(this.model.get('preventDuplicates')){
          recipient.get('cantBuyFor').splice(0,0, buyer.get('guid'));
        }else{
          return false;
        }
      },

      shuffle: function(){

        var boughtFor = [];

        this.collection.each(function(buyer, idx){

          // get all the valid recipients for this buyer as a backbone collection
          // NB. Valid recipients is not this.collection, it is a clone
          var validRecipients = this.validRecipients(buyer, boughtFor);

          if(validRecipients.length){

            // remove the recipient from the hat and match it to the actual human recipient
            var recipientTicket = validRecipients.at(this.randomDraw(validRecipients.length));
            var recipient = this.collection.get(recipientTicket.get('guid'));

            // give the recipient to the buyer
            buyer.set('buysFor', recipient.toJSON());

            this.preventDuplicatesDraw(buyer, recipient);

            // Set aside the recipients ticket
            boughtFor.push(recipient.get('guid'));  

          }else{

            this.makeSwap(buyer);       
          }

        }, this);

        return this.collection;

      },


      makeSwap : function(buyer){


        // Pick out a person to swap with at random (not the last)
        var validSwaps = this.validRecipients(buyer, []);

        var randomSwapper = this.collection.at(this.randomDraw(validSwaps.length));

        // Temporarily leave one person to buy for themselves
        buyer.set('buysFor', randomSwapper.get('buysFor'));

        // Swap with someone
        randomSwapper.set('buysFor', buyer.toJSON());
      },



      reShuffle: function(){
        // Shuffle reassign the pairs and reset the collection
        // Resetting the collection will rerender the list
        var reShuffled = this.shuffle();
        this.collection.reset(reShuffled.toJSON());
      }

    });


    var RootView = Mn.LayoutView.extend({

      el: '#app',

      regions : {
        "participantList": "#participant-list"
      },

      initialize: function() {
        // pass the collection down to the composite view
        this.getRegion('participantList').show(new AllParticipantsView({
          collection : this.collection
        }));
      }
    });



    var usersColl = new UsersCollection();

    var app = new Mn.Application({
        container : 'body',
        rootView :  new RootView({collection:usersColl})
    });

    usersColl.fetch().then(function(){
        app.start();
    });

    })();


</script>
\$\endgroup\$
1
1
\$\begingroup\$

If I was evaluating this code as a sample for an interview, I might be deterred by the inconsistent indentation - some blocks are indented with two spaces while others have four spaces. I do like the usage of templates and promises.

One suggestion I have would be to consider eliminating the extra lambda function when starting the application:

usersColl.fetch().then(function(){
    app.start();
});

And use a reference to the start method as the promise callback:

usersColl.fetch().then(app.start);

This may complicate matters because the method would receive as arguments any parameters returned from the promise. If that becomes an issue, maybe a partially bound function would help.

\$\endgroup\$

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.