Submission for C++ Readability

chromeos/dbus/fake_bluetooth_profile_manager_client.cc

 // Copyright (c) 2013 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 "chromeos/dbus/fake_bluetooth_profile_manager_client.h"
 
 

[omoikane, 2015/05/22 23:55:26]

Only 1 blank line needed here.

[Marie Janssen, 2015/06/18 21:08:34]

Done.

 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "chromeos/dbus/fake_bluetooth_profile_service_provider.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
 #include "dbus/object_proxy.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
@@ skipping 18 matching lines @@
     const std::string& uuid,
     const Options& options,
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
   VLOG(1) << "RegisterProfile: " << profile_path.value() << ": " << uuid;
 
   if (uuid == kUnregisterableUuid) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE, base::Bind(error_callback,
                               bluetooth_profile_manager::kErrorInvalidArguments,
                               "Can't register this UUID"));

[omoikane, 2015/05/22 23:55:26]

Current format makes the comma that separates the two arguments to PostTask less
visible, try something like this instead:

    base::MessageLoop::current()->PostTask(
        FROM_HERE,
        base::Bind(error_callback,
                   bluetooth_profile_manager::kErrorInvalidArguments,
                   "Can't register this UUID"));

[Marie Janssen, 2015/06/18 21:08:34]

Done.

     return;
   }
 
   // check options for channel & psm

[omoikane, 2015/05/22 23:55:26]

Is this a TODO comment?  If so, consider formatting it as such:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#TODO_Comments

The lines below don't seem to be related to "channel" and "psm".

[Marie Janssen, 2015/06/18 21:08:34]

Done.

 
   ServiceProviderMap::iterator iter = service_provider_map_.find(profile_path);
   if (iter == service_provider_map_.end()) {
     error_callback.Run(bluetooth_profile_manager::kErrorInvalidArguments,
                        "No profile created");
   } else {
     ProfileMap::iterator piter = profile_map_.find(uuid);
     if (piter != profile_map_.end()) {
       error_callback.Run(bluetooth_profile_manager::kErrorAlreadyExists,
                          "Profile already registered");
@@ skipping 38 matching lines @@
       service_provider_map_.find(service_provider->object_path_);
   if (iter != service_provider_map_.end() && iter->second == service_provider)
     service_provider_map_.erase(iter);
 }
 
 FakeBluetoothProfileServiceProvider*
 FakeBluetoothProfileManagerClient::GetProfileServiceProvider(
     const std::string& uuid) {
   ProfileMap::iterator iter = profile_map_.find(uuid);
   if (iter == profile_map_.end())
     return NULL;

[omoikane, 2015/05/22 23:55:26]

nullptr is preferred over NULL, since nullptr provides a bit more type safety.
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#0_and_nullptr...

(4 more files like this)

[Marie Janssen, 2015/06/18 21:08:34]

Done.

   return service_provider_map_[iter->second];
 }
 
 }  // namespace chromeos
+