Responding to comments from reviewer @kara

Author: ppham27

src/demo-app/select/select-demo.html

@@ -96,6 +96,32 @@
     </md-card-content>
   </md-card>
 
+
+  <md-card>
+    <md-card-subtitle>compareWith</md-card-subtitle>
+    <md-card-content>
+      <md-select placeholder="Drink" [color]="drinksTheme"
+                 [ngModel]="currentDrinkObject"
+                 (ngModelChange)="setDrinkObjectByCopy($event)"
+                 [required]="drinkObjectRequired"
+                 [compareWith]="compareByValue ? compareDrinkObjectsByValue : compareByReference"
+                 #drinkObjectControl="ngModel">
+        <md-option *ngFor="let drink of drinks" [value]="drink" [disabled]="drink.disabled">
+          {{ drink.viewValue }}
+        </md-option>
+      </md-select>
+      <p> Value: {{ currentDrinkObject | json }} </p>
+      <p> Touched: {{ drinkObjectControl.touched }} </p>
+      <p> Dirty: {{ drinkObjectControl.dirty }} </p>
+      <p> Status: {{ drinkObjectControl.control?.status }} </p>
+      <p> Comparison Mode: {{ compareByValue ? 'VALUE' : 'REFERENCE' }} </p>
+
+      <button md-button (click)="drinkObjectRequired=!drinkObjectRequired">TOGGLE REQUIRED</button>
+      <button md-button (click)="compareByValue=!compareByValue">TOGGLE COMPARE BY VALUE</button>
+      <button md-button (click)="drinkObjectControl.reset()">RESET</button>
+    </md-card-content>
+  </md-card>
+
   <div *ngIf="showSelect">
     <md-card>
       <md-card-subtitle>formControl</md-card-subtitle>
@@ -129,32 +155,5 @@
     </md-card>
   </div>
 
-  <div *ngIf="showSelect">
-    <md-card>
-      <md-card-subtitle>compareWith</md-card-subtitle>
-      <md-card-content>
-        <md-select placeholder="Drink" [color]="drinksTheme"
-                   [ngModel]="currentDrinkObject"
-                   (ngModelChange)="setDrinkObjectByCopy($event)"
-                   [required]="drinkObjectRequired"
-                   [compareWith]="compareByValue ? compareDrinkObjectsByValue : compareByReference"
-                   #drinkObjectControl="ngModel">
-          <md-option *ngFor="let drink of drinks" [value]="drink" [disabled]="drink.disabled">
-            {{ drink.viewValue }}
-          </md-option>
-        </md-select>
-        <p> Value: {{ currentDrinkObject | json }} </p>
-        <p> Touched: {{ drinkObjectControl.touched }} </p>
-        <p> Dirty: {{ drinkObjectControl.dirty }} </p>
-        <p> Status: {{ drinkObjectControl.control?.status }} </p>
-        <p> Comparison Mode: {{ compareByValue ? 'VALUE' : 'REFERENCE' }} </p>
-
-        <button md-button (click)="drinkObjectRequired=!drinkObjectRequired">TOGGLE REQUIRED</button>
-        <button md-button (click)="compareByValue=!compareByValue">TOGGLE COMPARE BY VALUE</button>
-        <button md-button (click)="drinkObjectControl.reset()">RESET</button>
-      </md-card-content>
-    </md-card>
-  </div>
-
 </div>
 <div style="height: 500px">This div is for testing scrolled selects.</div>

src/lib/select/select-errors.ts

@@ -7,8 +7,8 @@
  */
 
 /**
- * Returns an exception to be thrown when attempting to change a s
- * elect's `multiple` option after initialization.
+ * Returns an exception to be thrown when attempting to change a select's `multiple` option
+ * after initialization.
  * @docs-private
  */
 export function getMdSelectDynamicMultipleError(): Error {
@@ -24,3 +24,12 @@ export function getMdSelectDynamicMultipleError(): Error {
 export function getMdSelectNonArrayValueError(): Error {
   return Error('Cannot assign truthy non-array value to select in `multiple` mode.');
 }
+
+/**
+ * Returns an exception to be thrown when assigning a non-function value to the comparator
+ * used to determine if a value corresponds to an option. Note that whether the function
+ * actually takes two values and returns a boolean is not checked.
+ */
+export function getMdSelectNonFunctionValueError(): Error {
+  return Error('Cannot assign a non-function value to `compareWith`.');
+}

src/lib/select/select.spec.ts

@@ -28,12 +28,17 @@ import {Subject} from 'rxjs/Subject';
 import {map} from 'rxjs/operator/map';
 import {MdSelectModule} from './index';
 import {MdSelect} from './select';
-import {getMdSelectDynamicMultipleError, getMdSelectNonArrayValueError} from './select-errors';
+import {
+  getMdSelectDynamicMultipleError,
+  getMdSelectNonArrayValueError,
+  getMdSelectNonFunctionValueError
+} from './select-errors';
 import {MdOption} from '../core/option/option';
 import {
   FloatPlaceholderType,
   MD_PLACEHOLDER_GLOBAL_OPTIONS
 } from '../core/placeholder/placeholder-options';
+import {extendObject} from '../core/util/object-extend';
 
 
 describe('MdSelect', () => {
@@ -2728,20 +2733,24 @@ describe('MdSelect', () => {
       fixture.detectChanges();
     }));
 
-    it('should have a selection', () => {
-      const selectedOption = instance.select.selected as MdOption;
-      expect(selectedOption.value.value).toEqual('pizza-1');
-    });
+    describe('when comparing by value', () => {
 
-    it('should update when making a new selection', async(() => {
-      instance.options.last._selectViaInteraction();
-      fixture.detectChanges();
-      fixture.whenStable().then(() => {
+      it('should have a selection', () => {
         const selectedOption = instance.select.selected as MdOption;
-        expect(instance.selectedFood.value).toEqual('tacos-2');
-        expect(selectedOption.value.value).toEqual('tacos-2');
+        expect(selectedOption.value.value).toEqual('pizza-1');
       });
-    }));
+
+      it('should update when making a new selection', async(() => {
+        instance.options.last._selectViaInteraction();
+        fixture.detectChanges();
+        fixture.whenStable().then(() => {
+          const selectedOption = instance.select.selected as MdOption;
+          expect(instance.selectedFood.value).toEqual('tacos-2');
+          expect(selectedOption.value.value).toEqual('tacos-2');
+        });
+      }));
+
+    });
 
     describe('when comparing by reference', () => {
       beforeEach(async(() => {
@@ -2759,7 +2768,7 @@ describe('MdSelect', () => {
         expect(instance.select.selected).toBeUndefined();
       });
 
-      it('should not update the selection when changing the value', async(() => {
+      it('should not update the selection if value is copied on change', async(() => {
         instance.options.first._selectViaInteraction();
         fixture.detectChanges();
         fixture.whenStable().then(() => {
@@ -2778,7 +2787,7 @@ describe('MdSelect', () => {
       it('should throw an error', () => {
         expect(() => {
           fixture.detectChanges();
-        }).toThrowError('compareWith must be a function, but received null');
+        }).toThrowError(wrappedErrorMessage(getMdSelectNonFunctionValueError()));
       });
 
     });
@@ -3375,6 +3384,6 @@ class NgModelCompareWithSelect {
   compareByReference(f1: any, f2: any) { return f1 === f2; }
 
   setFoodByCopy(newValue: {value: string, viewValue: string}) {
-    this.selectedFood = Object.assign({}, newValue);
+    this.selectedFood = extendObject({}, newValue);
   }
 }

src/lib/select/select.ts

@@ -52,7 +52,11 @@ import {Observable} from 'rxjs/Observable';
 import {Subscription} from 'rxjs/Subscription';
 import {fadeInContent, transformPanel, transformPlaceholder} from './select-animations';
 import {SelectionModel} from '../core/selection/selection';
-import {getMdSelectDynamicMultipleError, getMdSelectNonArrayValueError} from './select-errors';
+import {
+  getMdSelectDynamicMultipleError,
+  getMdSelectNonArrayValueError,
+  getMdSelectNonFunctionValueError
+} from './select-errors';
 import {CanColor, mixinColor} from '../core/common-behaviors/color';
 import {CanDisable, mixinDisabled} from '../core/common-behaviors/disabled';
 import {MdOptgroup, MdOption, MdOptionSelectionChange} from '../core/option/index';
@@ -350,8 +354,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
   get compareWith() { return this._compareWith; }
   set compareWith(fn: (o1: any, o2: any) => boolean) {
     if (typeof fn !== 'function') {
-      throw new TypeError(
-        `compareWith must be a function, but received ${JSON.stringify(fn)}`);
+      throw getMdSelectNonFunctionValueError();
     }
     this._compareWith = fn;
     if (this._selectionModel) {
@@ -753,7 +756,7 @@ export class MdSelect extends _MdSelectMixinBase implements AfterContentInit, On
       isUserInput ? correspondingOption._selectViaInteraction() : correspondingOption.select();
       this._selectionModel.select(correspondingOption);
     }
-      
+
     return correspondingOption;
   }