0
\$\begingroup\$

I've got a Backbone.Collection which looks like this:

define([
    'background/mixin/collectionMultiSelect',
    'background/model/searchResult'
], function (CollectionMultiSelect, SearchResult) {
    'use strict';

    var SearchResults = Backbone.Collection.extend({
        model: SearchResult,

        mixins: [CollectionMultiSelect],

        //  Reset the collection's values by mapping a single or list of Song objects into
        //  JSON objects which will be instantiated by Backbone.
        setFromSongs: function (songs) {
            var searchResults = [];

            if (songs instanceof Backbone.Collection) {
                searchResults = songs.map(function (song) {
                    return {
                        song: song,
                        title: song.get('title')
                    };
                });
            } else if (_.isArray(songs)) {
                searchResults = _.map(songs, function (song) {
                    return {
                        song: song,
                        title: song.get('title')
                    };
                });
            } else {
                searchResults.push({
                    song: songs,
                    title: songs.get('title')
                });
            }

            this.reset(searchResults);
        },

        //  Returns the underlying Songs of the selected SearchResults.
        getSelectedSongs: function() {
            return _.map(this.selected(), function (searchResult) {
                return searchResult.get('song');
            });
        }
    });

    return SearchResults;
});

I feel like I'm being overly verbose with my setFromSongs method. The intent of the method is to allow outside objects to dumbly give a single Song object, an array of Song objects, or a Backbone.Collection of Songs to the method and have it handle them appropriately.

I thought that this made sense for a few reasons:

  • Backbone already anticipates singular versus multiple objects with its add and reset methods. I would like my methods to do so, as well.

  • When working with a Collection it often becomes an array through use of underscore utility methods such as map, pluck, etc.

Is this as concise as this method can be while still making its intent clear? How would you approach this?

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

setFromSongs initialises the searchResults variable and then in 2-of-3 cases it immediately overwrites it. You can rewrite the singular case to create the array containing only the single object then you don't need to initialise the variable immediately.

You are repeating the code { song: x, title: x.get('title') } in three different places - you can make the code more DRY if you create a function defined outside the class to handle the mapping of a song to its association with its title:

var associateSongWithTitle = function( song ){
  return {
    song:  song,
    title: song.get('title')
  };
};

You can then refactor the code to:

var SearchResults = Backbone.Collection.extend({
  ...
  setFromSongs: function (songs) {
      var searchResults;
      if (songs instanceof Backbone.Collection) {
          searchResults = songs.map( associateSongWithTitle );
      } else if (_.isArray(songs)) {
          searchResults = _.map(songs, associateSongWithTitle );
      } else {
          searchResults = [ associateSongWithTitle( songs ) ];
      }
      this.reset(searchResults);
  },
  ...
});
\$\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.