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?

asked Nov 6, 2014 at 20:29
\$\endgroup\$

1 Answer 1

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);
 },
 ...
});
answered Nov 10, 2014 at 20:58
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.