Integrating Online Wallet into Chrome.

chrome/browser/autofill/wallet/wallet_address.cc

+// 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.
+
+#include "chrome/browser/autofill/wallet/wallet_address.h"
+
+#include "base/logging.h"
+#include "base/values.h"
+
+namespace wallet {
+
+Address::Address() {}
+
+Address::Address(const std::string& country_name_code,
+        const std::string& recipient_name,

[Dan Beam, 2012/11/30 19:55:52]

nit: line up with (, i.e.

  Address::Address(const std::string& country_name_code,
                   const std::string& recipient_name,

[ahutter, 2012/12/01 04:06:51]

Done.

+        const std::string& address_line_1,
+        const std::string& address_line_2,
+        const std::string& locality_name,
+        const std::string& administrative_area_name,
+        const std::string& postal_code_number,
+        const std::string& phone_number,
+        const std::string& object_id)
+    : country_name_code_(country_name_code),
+      recipient_name_(recipient_name),
+      address_line_1_(address_line_1),
+      address_line_2_(address_line_2),
+      locality_name_(locality_name),
+      administrative_area_name_(administrative_area_name),
+      postal_code_number_(postal_code_number),
+      phone_number_(phone_number),
+      object_id_(object_id) {}
+
+Address::~Address() {}

[Dan Beam, 2012/11/30 19:55:52]

nit: make wallet::Cart::~Cart() into

  Cart::~Cart() {}

or make this

  Address::~Address() {
  }

IMO

[ahutter, 2012/12/01 04:06:51]

Done.

+
+Address* Address::CreateIdedAddress(

[Dan Beam, 2012/11/30 19:55:52]

can this be CreateAddressWithID to avoid strange "Ided"?

[ahutter, 2012/12/01 04:06:51]

Done.

+    base::DictionaryValue* dictionary) {
+  DCHECK(dictionary);
+
+  std::string object_id;
+  if (!dictionary->GetString("id", &object_id)) {
+    DLOG(ERROR) << "Response from Google Wallet missing object id";
+    return NULL;
+  }
+  std::string phone_number;
+  if (!dictionary->GetString("phone_number", &phone_number)) {
+    VLOG(1) << "Response from Google Wallet missing phone number";

[Dan Beam, 2012/11/30 19:55:52]

nit: why is this VLOG(1) vs DLOG()? also, why not DVLOG()?

[ahutter, 2012/12/01 04:06:51]

Now DVLOG.  It's not DLOG because it's good for debugging to know if this
happens but not necessary all the time.

+  }
+  std::string country_name_code;
+  if (!dictionary->GetString("postal_address.country_name_code",
+                              &country_name_code)) {
+    DLOG(ERROR) << "Response from Google Wallet missing country name";
+    return NULL;
+  }
+  std::string recipient_name;
+  if (!dictionary->GetString("postal_address.recipient_name",
+                             &recipient_name)) {
+    DLOG(ERROR) << "Response from Google Wallet recipient name";
+    return NULL;
+  }
+  std::string address_line_1;
+  std::string address_line_2;
+  ListValue* address_line_list;
+  if (dictionary->GetList("postal_address.address_line", &address_line_list)) {
+    if (!address_line_list->GetString(0, &address_line_1))
+      VLOG(1) << "Response from Google Wallet missing address line 1";
+    if (!address_line_list->GetString(1, &address_line_2))
+      VLOG(1) << "Response from Google Wallet missing address line 2";
+  } else {
+    VLOG(1) << "Response from Google Wallet missing address lines";
+  }
+  std::string locality_name;
+  if (!dictionary->GetString("postal_address.locality_name",
+                             &locality_name)) {
+    VLOG(1) << "Response from Google Wallet missing locality name";
+  }
+  std::string administrative_area_name;
+  if (!dictionary->GetString("postal_address.administrative_area_name",
+                             &administrative_area_name)) {
+    VLOG(1) << "Response from Google Wallet missing administrative area name";
+  }
+  std::string postal_code_number;
+  if (!dictionary->GetString("postal_address.postal_code_number",
+                              &postal_code_number)) {
+    DLOG(ERROR) << "Response from Google Wallet missing postal code number";
+    return NULL;
+  }

[Dan Beam, 2012/11/30 19:55:52]

nit: needs moar \n's, IMO

[ahutter, 2012/12/01 04:06:51]

Done.

+
+  return new Address(country_name_code,
+                     recipient_name ,
+                     address_line_1,
+                     address_line_2,
+                     locality_name,
+                     administrative_area_name,
+                     postal_code_number,
+                     phone_number,
+                     object_id);
+}
+
+
+Address* Address::CreateDisplayAddress(
+    base::DictionaryValue* dictionary) {

[Dan Beam, 2012/11/30 19:55:52]

nit: is there any reason these params can't be const ref and these methods can't
be const?

[ahutter, 2012/12/01 04:06:51]

Params can be const ref but the methods are static.

+  DCHECK(dictionary);
+  std::string country_code;
+
+  if (!dictionary->GetString("country_code", &country_code)) {
+    DLOG(ERROR) << "Reponse from Google Wallet missing country code";
+    return NULL;
+  }
+  std::string name;
+  if (!dictionary->GetString("name", &name)) {
+    DLOG(ERROR) << "Reponse from Google Wallet missing name";
+    return NULL;
+  }
+  std::string address1;
+  if (!dictionary->GetString("address1", &address1))
+    VLOG(1) << "Reponse from Google Wallet missing address1";
+  std::string address2;
+  if (!dictionary->GetString("address2", &address2))
+    VLOG(1) << "Reponse from Google Wallet missing address2";
+  std::string city;
+  if (!dictionary->GetString("city", &city))
+    VLOG(1) << "Reponse from Google Wallet missing city";
+  std::string state;
+  if (!dictionary->GetString("state", &state))
+    VLOG(1) << "Reponse from Google Wallet missing state";
+  std::string postal_code;
+  if (!dictionary->GetString("postal_code", &postal_code)) {
+    DLOG(ERROR) << "Reponse from Google Wallet missing postal code";
+    return NULL;
+  }
+  std::string phone_number;
+  if (!dictionary->GetString("phone_number", &phone_number))
+    VLOG(1) << "Reponse from Google Wallet missing phone number";
+
+  return new Address(country_code,
+                     name,
+                     address1,
+                     address2,
+                     city,
+                     state,
+                     postal_code,
+                     phone_number,
+                     "");
+}
+
+bool Address::operator==(const Address& other) const {
+  if (country_name_code_.compare(other.country_name_code_) != 0)

[Dan Beam, 2012/11/30 19:55:52]

nit: why are you using compare instead of ==? this seems to be shorter and
easier to understand (and not mess up), IMO

  return country_name_code_ == other.country_name_code_ &&
         recipient_name_ == other.recipient_name_ &&
         address_line_1_ == other.address_line_1_ &&
         address_line_2_ == other.address_line_2_ &&
         administrative_area_name_ == other.administrative_area_name_ &&
         postal_code_number_ == other.postal_code_number_ &&
         phone_number_ == other.phone_number_ &&
         object_id_ == other.object_id_;

[ahutter, 2012/12/01 04:06:51]

Done.

+    return false;
+  if (recipient_name_.compare(other.recipient_name_) != 0)
+    return false;
+  if (address_line_1_.compare(other.address_line_1_) != 0)
+    return false;
+  if (address_line_2_.compare(other.address_line_2_) != 0)
+    return false;
+  if (locality_name_.compare(other.locality_name_) != 0)
+    return false;
+  if (administrative_area_name_.compare(other.administrative_area_name_) != 0)
+    return false;
+  if (postal_code_number_ != other.postal_code_number_)
+    return false;
+  if (phone_number_.compare(other.phone_number_) != 0)
+    return false;
+  if (object_id_.compare(other.object_id_) != 0)
+    return false;
+  return true;
+}
+
+bool Address::operator!=(const Address& other) const {
+  return !(*this == other);
+}
+
+
+}  // end wallet namespace