Add ContentSuggestionsCategoryFactory; Store categories as ints

components/ntp_snippets/content_suggestions_category_factory.h

+// Copyright 2016 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 COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_CATEGORY_FACTORY_H_
+#define COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_CATEGORY_FACTORY_H_
+
+#include <map>
+#include <vector>
+
+#include "base/macros.h"
+#include "components/ntp_snippets/content_suggestions_category.h"
+
+namespace ntp_snippets {
+
+// Creates and orders instances of ContentSuggestionsCategory.
+class ContentSuggestionsCategoryFactory {
+ public:
+  ContentSuggestionsCategoryFactory();
+  ~ContentSuggestionsCategoryFactory();
+
+  // Creates a category from a KnownSuggestionsCategory value. The passed
+  // |known_category| must not be one of the special values
+  // (LOCAL_CATEGORIES_COUNT or REMOTE_CATEGORIES_OFFSET).
+  ContentSuggestionsCategory FromKnownCategory(
+      KnownSuggestionsCategories known_category);
+
+  // Creates a category from a category ID delivered by the server. |remote_id|
+  // must be positive. The resulting category will be
+  // |REMOTE_CATEGORIES_OFFSET + remote_id|. It may, but does not have to match

[tschumann, 2016/07/28 15:36:10]

IMO, the factory should not document this detail (about how the category id is
computed for remote categories) about how we represent the categories. That's a
detail that should be only known by the category and factory implementation.

[Philipp Keck, 2016/07/28 16:59:58]

Yeah, this might be the wrong place to document this. It's not totally an
implementation detail though because the same relation is hard-coded in
KnownSuggestionsCategories for at least some of those categories.
I removed the comment here, it should become clear there anyway and it's only
relevant there (to keep in sync with the enum on the server).

[tschumann, 2016/07/28 17:05:16]

Thanks. Arguable the enum constant values should also be an implementation
details, there's just no way to hide them =).
The coupling between the factory's implementation and the enum values is
completely fine IMO, but clients of the factory should not make such assumptions

+  // a KnownSuggestionsCategory.
+  ContentSuggestionsCategory FromRemoteCategory(int remote_id);
+
+  // Creates a category from an ID as returned by
+  // |ContentSuggestionsCategory::id()|. |id| must be a non-negative value.
+  ContentSuggestionsCategory FromIDValue(int id);
+
+  // Compares the given categories according to a strict ordering, returning
+  // true if and only if |left| is strictly less than |right|.
+  // This method satisfies the "Compare" contract required by sort algorithms.
+  bool CompareCategories(const ContentSuggestionsCategory& left,
+                         const ContentSuggestionsCategory& right) const;
+
+ private:
+  bool CategoryExists(int id);
+  void AddKnownCategory(KnownSuggestionsCategories known_category);
+  ContentSuggestionsCategory InternalFromID(int id);
+
+  // Stores all known categories in the order which is also returned by
+  // |CompareCategories|.
+  std::vector<ContentSuggestionsCategory> categories_;

[tschumann, 2016/07/28 15:36:10]

should we rename this to ordered_categories_? Just a thought, not feeling
strongly.

[Philipp Keck, 2016/07/28 16:59:58]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ContentSuggestionsCategoryFactory);
+};
+
+}  // namespace ntp_snippets
+
+#endif  // COMPONENTS_NTP_SNIPPETS_CONTENT_SUGGESTIONS_CATEGORY_FACTORY_H_