[CrOS Tether] Sort Tether network lists.

chromeos/components/tether/host_scan_device_prioritizer_impl.h

+// Copyright 2017 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.
+
+#ifndef CHROMEOS_COMPONENTS_TETHER_HOST_SCAN_DEVICE_PRIORITIZER_IMPL_H_
+#define CHROMEOS_COMPONENTS_TETHER_HOST_SCAN_DEVICE_PRIORITIZER_IMPL_H_
+
+#include "base/macros.h"
+#include "chromeos/components/tether/host_scan_device_prioritizer.h"
+#include "chromeos/network/network_state_handler.h"
+#include "components/cryptauth/remote_device.h"
+
+namespace chromeos {
+
+class NetworkStateHandler;
+
+namespace tether {
+
+class TetherHostResponseRecorder;
+class DeviceIdTetherNetworkGuidMap;
+
+// Implementation of HostScanDevicePrioritizer.
+class HostScanDevicePrioritizerImpl
+    : public HostScanDevicePrioritizer,
+      public NetworkStateHandler::TetherNetworkListSorter {
+ public:
+  HostScanDevicePrioritizerImpl(
+      NetworkStateHandler* network_state_handler,
+      TetherHostResponseRecorder* tether_host_response_recorder,
+      DeviceIdTetherNetworkGuidMap* device_id_tether_network_guid_map);
+  ~HostScanDevicePrioritizerImpl() override;
+
+  // HostScanDevicePrioritizer:
+  void SortByHostScanOrder(
+      std::vector<cryptauth::RemoteDevice>* remote_devices) const override;
+
+  // NetworkStateHandler::TetherNetworkListSorter:
+  void SortTetherNetworkList(
+      NetworkStateHandler::ManagedStateList* tether_networks) const override;
+
+ private:
+  // Performs sorting for both SortByHostScanOrder() and
+  // SortTetherNetworkList().
+  template <typename T>
+  void SortNetworks(std::vector<T>* list_to_sort) const;
+
+  // GetDeviceId() and Move() are used internally by SortNetworks() to sort
+  // lists of both cryptauth::RemoteDevice and std::unique_ptr<ManagedState>.
+  std::string GetDeviceId(const cryptauth::RemoteDevice& remote_device) const;
+  std::string GetDeviceId(
+      const std::unique_ptr<ManagedState>& tether_network_state) const;
+  cryptauth::RemoteDevice Move(
+      const cryptauth::RemoteDevice& remote_device) const;
+  std::unique_ptr<ManagedState> Move(
+      std::unique_ptr<ManagedState>& tether_network_state) const;

[stevenjb, 2017/06/21 20:59:03]

All of the above is a lot of detail for a header (even for an Impl). Would it be
reasonable to create a NetworkSorter template class (defined entirely in the .cc
file) instead?

[Kyle Horimoto, 2017/06/21 22:17:07]

I moved both Move() implementations to an anonymous namespace in the .cc. I left
the GetDeviceId() implementations since they require instance fields.

I'm not exactly sure what you meant about the NetworkSorter template class, but
I think this is sufficient cleanup of the .h file.

[stevenjb, 2017/06/22 00:25:19]

OK, I looked at this a little more closely and see what's going on, kind of...
it's a little odd using the same sorting method for two very different lists,
but as long as it is doing what it is supposed to, it seems fine.

If GetDeviceId(ManagedState*) didn't require the
device_id_tether_network_guid_map_ lookup (e.g. if it could get the device id
from the ManagedState), I would suggest making the GetDeviceId methods file
local, then pass tether_host_response_recorder_ to SortNetworks() which could
then be a file local template function which would be a little simpler, but this
is fine.

[Kyle Horimoto, 2017/06/22 02:08:01]

Acknowledged.

+
+  NetworkStateHandler* network_state_handler_;
+  TetherHostResponseRecorder* tether_host_response_recorder_;
+  DeviceIdTetherNetworkGuidMap* device_id_tether_network_guid_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostScanDevicePrioritizerImpl);
+};
+
+}  // namespace tether
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_TETHER_HOST_SCAN_DEVICE_PRIORITIZER_IMPL_H_