Refactor print preview web ui

chrome/browser/resources/print_preview/data/destination.js

Index: chrome/browser/resources/print_preview/data/destination.js
diff --git a/chrome/browser/resources/print_preview/data/destination.js b/chrome/browser/resources/print_preview/data/destination.js
new file mode 100644
index 0000000000000000000000000000000000000000..734878dbb5cb5f9799a75d5eec5d127eb76735ed
--- /dev/null
+++ b/chrome/browser/resources/print_preview/data/destination.js
@@ -0,0 +1,170 @@
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cr.define('print_preview', function() {
+  'use strict';
+
+  /**
+   * Destination data object.
+   *
+   * This data object holds data for both local and cloud destinations.
+   *
+   * @param {string} id ID of the destination.
+   * @param {string} displayName Display name of the destination.
+   * @param {boolean} isRecent Whether the destination has been used recently.
+   * @param {boolean} isLocal Whether the destination is local or cloud-based.
+   * @param {Array.<string>} tags Tags associated with the destination.

[dpapad1, 2012/04/19 22:45:01]

If this is optional (seems like it is judging from line 54) use
@param{Array.<string>=}

[Robert Toscano, 2012/04/23 23:00:09]

Done.

+   * @constructor
+   */
+  function Destination(id, displayName, isRecent, isLocal, tags) {
+    /**
+     * ID of the destination.
+     * @type {string}
+     * @private
+     */
+    this.id_ = id;
+
+    /**
+     * Display name of the destination.
+     * @type {string}
+     * @private
+     */
+    this.displayName_ = displayName;
+
+    /**
+     * Whether the destination has been used recently.
+     * @type {boolean}
+     * @private
+     */
+    this.isRecent_ = isRecent;
+
+    /**
+     * Whether the destination is local or cloud-based.

[dpapad1, 2012/04/19 22:45:01]

Nit (optional): "{@code true}  if destination is local, {@code false}  if
cloud-based."

[Robert Toscano, 2012/04/23 23:00:09]

I'll leave as is. FWIW, "Whether ...." is a bit more concise to me.

+     * @type {boolean}
+     * @private
+     */
+    this.isLocal_ = isLocal;
+
+    /**
+     * Tags associated with the destination.
+     * @type {Array.<string>}
+     * @private
+     */
+    this.tags_ = tags || [];
+
+    /**
+     * Print capabilities of the destination.
+     * @type {print_preview.ChromiumCapabilities}
+     * @private
+     */
+    this.capabilities_ = null;
+
+    /**
+     * Cache of destination location fetched from tags.
+     * @type {string}
+     * @private
+     */
+    this.location_ = null;
+  };
+
+  /**
+   * Prefix of the location destination tag.
+   * @type {string}
+   * @const
+   */
+  Destination.LOCATION_TAG_PREFIX = '__cp__printer-location=';
+
+  /**
+   * Enumeration of Google-promoted destination IDs.
+   * @enum {string}
+   * @private
+   */
+  Destination.GooglePromotedIds_ = {
+    DOCS: '__google__docs',
+    LOCAL_PDF: PRINT_TO_PDF,
+    PRINT_WITH_CLOUD_PRINT: 'printWithCloudPrint'
+  };
+
+  Destination.prototype = {
+    get id() {
+      return this.id_;
+    },
+
+    get displayName() {
+      return this.displayName_;
+    },
+
+    get isRecent() {
+      return this.isRecent_;
+    },
+
+    set isRecent(isRecent) {
+      this.isRecent_ = isRecent;
+    },
+
+    get isLocal() {
+      return this.isLocal_;
+    },
+
+    get isGooglePromoted() {

[dpapad1, 2012/04/19 22:45:01]

I believe is better to use the "get foo() {...}" syntax only if you are directly
returning a member var, so in this case i would use regular method syntax (foo:
function() {...}), also for "get location()". You don't have to adhere just a
personal preference.

[Robert Toscano, 2012/04/23 23:00:09]

I agree with you on this point, but I'm also worried that it would be
inconsistent since most of the members of this class are simple getters. I was
struggling with this idea and decided it would make using the class easier if I
tried to keep the usage of getters (or lack of) consistent within a class.

+      for (var key in Destination.GooglePromotedIds_) {
+        if (Destination.GooglePromotedIds_[key] == this.id_) {
+          return true;
+        }
+      }
+      return false;
+    },
+
+    get isPrintToPdf() {
+      return this.id_ == Destination.GooglePromotedIds_.LOCAL_PDF;
+    },
+
+    get isPrintWithCloudPrint() {
+      return this.id_ == Destination.GooglePromotedIds_.PRINT_WITH_CLOUD_PRINT;
+    },
+
+    get location() {
+      if (this.location_ == null) {
+        for (var tag, i = 0; tag = this.tags_[i]; i++) {
+          if (tag.indexOf(Destination.LOCATION_TAG_PREFIX) == 0) {
+            this.location_ = tag.substring(
+                Destination.LOCATION_TAG_PREFIX.length);
+          }
+        }
+        if (this.location_ == null) {
+          this.location_ = '';
+        }
+      }
+      return this.location_;
+    },
+
+    get tags() {
+      return this.tags_.slice(0);
+    },
+
+    get capabilities() {
+      return this.capabilities_;
+    },
+
+    set capabilities(capabilities) {
+      this.capabilities_ = capabilities;
+    },
+
+    /**
+     * Matches a query against the destination.
+     * @param {string} query Query to match against the destination.
+     * @return {boolean} {@code true} if the query matches this destination,
+     *     {@code false} otherwise.
+     */
+    matches: function(query) {
+      return this.displayName_.toLowerCase().indexOf(
+          query.toLowerCase().trim()) != -1;
+    }
+  };
+
+  // Export
+  return {
+    Destination: Destination
+  };
+});