Skip to content

Commit 903c414

Browse files
committed
use matchers and switch to internal header
1 parent e34f3da commit 903c414

4 files changed

Lines changed: 160 additions & 129 deletions

File tree

src/iceberg/statistics_file.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
#include <format>
2323

24-
#include "iceberg/util/formatter.h"
24+
#include "iceberg/util/formatter_internal.h"
2525

2626
namespace iceberg {
2727

src/iceberg/util/formatter.h

Lines changed: 0 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,7 @@
2727

2828
#include <concepts>
2929
#include <format>
30-
#include <map>
3130
#include <string_view>
32-
#include <unordered_map>
33-
#include <vector>
3431

3532
#include "iceberg/util/formattable.h"
3633

@@ -44,93 +41,6 @@ struct std::formatter<Derived> : std::formatter<std::string_view> {
4441
}
4542
};
4643

47-
/// \brief std::formatter specialization for std::vector
48-
template <typename T>
49-
struct std::formatter<std::vector<T>> : std::formatter<std::string_view> {
50-
template <class FormatContext>
51-
auto format(const std::vector<T>& vec, FormatContext& ctx) const {
52-
std::string result = "[";
53-
54-
bool first = true;
55-
for (const auto& item : vec) {
56-
if (!first) {
57-
std::format_to(std::back_inserter(result), ", ");
58-
}
59-
if constexpr (requires { *item; }) {
60-
if (item) {
61-
std::format_to(std::back_inserter(result), "{}", *item);
62-
} else {
63-
std::format_to(std::back_inserter(result), "null");
64-
}
65-
} else {
66-
std::format_to(std::back_inserter(result), "{}", item);
67-
}
68-
first = false;
69-
}
70-
71-
std::format_to(std::back_inserter(result), "]");
72-
return std::formatter<std::string_view>::format(result, ctx);
73-
}
74-
};
75-
76-
/// \brief Helper template for formatting map-like containers
77-
template <typename MapType>
78-
std::string FormatMap(const MapType& map) {
79-
std::string result = "{";
80-
81-
bool first = true;
82-
for (const auto& [key, value] : map) {
83-
if (!first) {
84-
std::format_to(std::back_inserter(result), ", ");
85-
}
86-
87-
// Format key (handle if it's a smart pointer)
88-
if constexpr (requires { *key; }) {
89-
if (key) {
90-
std::format_to(std::back_inserter(result), "{}: ", *key);
91-
} else {
92-
std::format_to(std::back_inserter(result), "null: ");
93-
}
94-
} else {
95-
std::format_to(std::back_inserter(result), "{}: ", key);
96-
}
97-
98-
// Format value (handle if it's a smart pointer)
99-
if constexpr (requires { *value; }) {
100-
if (value) {
101-
std::format_to(std::back_inserter(result), "{}", *value);
102-
} else {
103-
std::format_to(std::back_inserter(result), "null");
104-
}
105-
} else {
106-
std::format_to(std::back_inserter(result), "{}", value);
107-
}
108-
109-
first = false;
110-
}
111-
112-
std::format_to(std::back_inserter(result), "}}");
113-
return result;
114-
}
115-
116-
/// \brief std::formatter specialization for std::map
117-
template <typename K, typename V>
118-
struct std::formatter<std::map<K, V>> : std::formatter<std::string_view> {
119-
template <class FormatContext>
120-
auto format(const std::map<K, V>& map, FormatContext& ctx) const {
121-
return std::formatter<std::string_view>::format(FormatMap(map), ctx);
122-
}
123-
};
124-
125-
/// \brief std::formatter specialization for std::unordered_map
126-
template <typename K, typename V>
127-
struct std::formatter<std::unordered_map<K, V>> : std::formatter<std::string_view> {
128-
template <class FormatContext>
129-
auto format(const std::unordered_map<K, V>& map, FormatContext& ctx) const {
130-
return std::formatter<std::string_view>::format(FormatMap(map), ctx);
131-
}
132-
};
133-
13444
/// \brief std::formatter specialization for any type that has a ToString function
13545
template <typename T>
13646
requires requires(const T& t) {
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#pragma once
21+
22+
#include <concepts>
23+
#include <format>
24+
#include <map>
25+
#include <ranges>
26+
#include <sstream>
27+
#include <string_view>
28+
#include <unordered_map>
29+
#include <vector>
30+
31+
#include "iceberg/util/formatter.h"
32+
33+
/// \brief Concept for smart pointer types
34+
template <typename T>
35+
concept SmartPointerType = requires(T t) {
36+
{ t.operator->() } -> std::same_as<typename T::element_type*>;
37+
{ *t } -> std::convertible_to<typename T::element_type&>;
38+
{ static_cast<bool>(t) } -> std::same_as<bool>;
39+
typename T::element_type;
40+
};
41+
42+
/// \brief Helper function to format an item using concepts to differentiate types
43+
template <typename T>
44+
std::string FormatItem(const T& item) {
45+
if constexpr (SmartPointerType<T>) {
46+
if (item) {
47+
return std::format("{}", *item);
48+
} else {
49+
return "null";
50+
}
51+
} else {
52+
return std::format("{}", item);
53+
}
54+
}
55+
56+
/// \brief Generic function to join a range of elements with a separator and wrap with
57+
/// delimiters
58+
template <std::ranges::input_range Range>
59+
std::string FormatRange(const Range& range, std::string_view separator,
60+
std::string_view prefix, std::string_view suffix) {
61+
if (std::ranges::empty(range)) {
62+
return std::format("{}{}", prefix, suffix);
63+
}
64+
65+
std::stringstream ss;
66+
ss << prefix;
67+
68+
bool first = true;
69+
for (const auto& element : range) {
70+
if (!first) {
71+
ss << separator;
72+
}
73+
ss << element;
74+
first = false;
75+
}
76+
77+
ss << suffix;
78+
return ss.str();
79+
}
80+
81+
/// \brief Helper template for formatting map-like containers
82+
template <typename MapType>
83+
std::string FormatMap(const MapType& map) {
84+
// Transform the map items into formatted key-value pairs
85+
std::ranges::transform_view formatted_range =
86+
map | std::views::transform([](const auto& pair) -> std::string {
87+
const auto& [key, value] = pair;
88+
return std::format("{}: {}", FormatItem(key), FormatItem(value));
89+
});
90+
return FormatRange(formatted_range, ", ", "{", "}");
91+
}
92+
93+
/// \brief std::formatter specialization for std::map
94+
template <typename K, typename V>
95+
struct std::formatter<std::map<K, V>> : std::formatter<std::string_view> {
96+
template <class FormatContext>
97+
auto format(const std::map<K, V>& map, FormatContext& ctx) const {
98+
return std::formatter<std::string_view>::format(FormatMap(map), ctx);
99+
}
100+
};
101+
102+
/// \brief std::formatter specialization for std::unordered_map
103+
template <typename K, typename V>
104+
struct std::formatter<std::unordered_map<K, V>> : std::formatter<std::string_view> {
105+
template <class FormatContext>
106+
auto format(const std::unordered_map<K, V>& map, FormatContext& ctx) const {
107+
return std::formatter<std::string_view>::format(FormatMap(map), ctx);
108+
}
109+
};
110+
111+
/// \brief std::formatter specialization for std::vector
112+
template <typename T>
113+
struct std::formatter<std::vector<T>> : std::formatter<std::string_view> {
114+
template <class FormatContext>
115+
auto format(const std::vector<T>& vec, FormatContext& ctx) const {
116+
auto formatted_range =
117+
vec | std::views::transform([](const auto& item) { return FormatItem(item); });
118+
return std::formatter<std::string_view>::format(
119+
FormatRange(formatted_range, ", ", "[", "]"), ctx);
120+
}
121+
};

test/formatter_test.cc

Lines changed: 38 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,19 @@
1717
* under the License.
1818
*/
1919

20-
#include "iceberg/util/formatter.h"
21-
2220
#include <format>
2321
#include <map>
2422
#include <memory>
2523
#include <string>
2624
#include <unordered_map>
2725
#include <vector>
2826

27+
#include <gmock/gmock-matchers.h>
28+
#include <gmock/gmock.h>
2929
#include <gtest/gtest.h>
3030

3131
#include "iceberg/statistics_file.h"
32+
#include "iceberg/util/formatter_internal.h"
3233

3334
namespace iceberg {
3435

@@ -59,9 +60,9 @@ TEST(FormatterTest, UnorderedMapFormat) {
5960
std::unordered_map<std::string, double> scores = {
6061
{"Alice", 95.5}, {"Bob", 87.0}, {"Charlie", 92.3}};
6162
std::string str = std::format("{}", scores);
62-
EXPECT_TRUE(str.find("Alice: 95.5") != std::string::npos);
63-
EXPECT_TRUE(str.find("Bob: 87") != std::string::npos);
64-
EXPECT_TRUE(str.find("Charlie: 92.3") != std::string::npos);
63+
EXPECT_THAT(str, ::testing::HasSubstr("Alice: 95.5"));
64+
EXPECT_THAT(str, ::testing::HasSubstr("Bob: 87"));
65+
EXPECT_THAT(str, ::testing::HasSubstr("Charlie: 92.3"));
6566
}
6667

6768
TEST(FormatterTest, NestedContainersFormat) {
@@ -73,10 +74,10 @@ TEST(FormatterTest, NestedContainersFormat) {
7374
std::map<std::string, std::vector<int>> nested_map = {
7475
{"primes", {2, 3, 5, 7, 11}}, {"fibonacci", {1, 1, 2, 3, 5, 8, 13}}};
7576
std::string result = std::format("{}", nested_map);
76-
EXPECT_TRUE(result.find("primes") != std::string::npos);
77-
EXPECT_TRUE(result.find("fibonacci") != std::string::npos);
78-
EXPECT_TRUE(result.find("[2, 3, 5, 7, 11]") != std::string::npos);
79-
EXPECT_TRUE(result.find("[1, 1, 2, 3, 5, 8, 13]") != std::string::npos);
77+
EXPECT_THAT(result, ::testing::HasSubstr("primes"));
78+
EXPECT_THAT(result, ::testing::HasSubstr("fibonacci"));
79+
EXPECT_THAT(result, ::testing::HasSubstr("[2, 3, 5, 7, 11]"));
80+
EXPECT_THAT(result, ::testing::HasSubstr("[1, 1, 2, 3, 5, 8, 13]"));
8081
}
8182

8283
TEST(FormatterTest, EdgeCasesFormat) {
@@ -91,42 +92,41 @@ TEST(FormatterTest, EdgeCasesFormat) {
9192
}
9293

9394
TEST(FormatterTest, SmartPointerFormat) {
94-
std::vector<std::shared_ptr<int>> int_ptrs;
95-
int_ptrs.push_back(std::make_shared<int>(42));
96-
int_ptrs.push_back(std::make_shared<int>(123));
97-
int_ptrs.push_back(nullptr);
95+
std::vector<std::shared_ptr<int>> int_ptrs = {
96+
std::make_shared<int>(42),
97+
std::make_shared<int>(123),
98+
nullptr,
99+
};
98100
EXPECT_EQ("[42, 123, null]", std::format("{}", int_ptrs));
99101

100-
std::vector<std::shared_ptr<std::string>> str_ptrs;
101-
str_ptrs.push_back(std::make_shared<std::string>("hello"));
102-
str_ptrs.push_back(std::make_shared<std::string>("world"));
103-
str_ptrs.push_back(nullptr);
102+
std::vector<std::shared_ptr<std::string>> str_ptrs = {
103+
std::make_shared<std::string>("hello"),
104+
std::make_shared<std::string>("world"),
105+
nullptr,
106+
};
104107
EXPECT_EQ("[hello, world, null]", std::format("{}", str_ptrs));
105108

106-
std::map<std::string, std::shared_ptr<int>> map_with_ptr_values;
107-
map_with_ptr_values["one"] = std::make_shared<int>(1);
108-
map_with_ptr_values["two"] = std::make_shared<int>(2);
109-
map_with_ptr_values["null"] = nullptr;
109+
std::map<std::string, std::shared_ptr<int>> map_with_ptr_values = {
110+
{"one", std::make_shared<int>(1)},
111+
{"two", std::make_shared<int>(2)},
112+
{"null", nullptr},
113+
};
110114
EXPECT_EQ("{null: null, one: 1, two: 2}", std::format("{}", map_with_ptr_values));
111115

112-
std::unordered_map<std::string, std::shared_ptr<double>> scores;
113-
scores["Alice"] = std::make_shared<double>(95.5);
114-
scores["Bob"] = std::make_shared<double>(87.0);
115-
scores["Charlie"] = nullptr;
116+
std::unordered_map<std::string, std::shared_ptr<double>> scores = {
117+
{"Alice", std::make_shared<double>(95.5)},
118+
{"Bob", std::make_shared<double>(87.0)},
119+
{"Charlie", nullptr},
120+
};
116121
std::string str = std::format("{}", scores);
117-
EXPECT_TRUE(str.find("Alice: 95.5") != std::string::npos);
118-
EXPECT_TRUE(str.find("Bob: 87") != std::string::npos);
119-
EXPECT_TRUE(str.find("Charlie: null") != std::string::npos);
120-
121-
std::vector<std::map<std::string, std::shared_ptr<int>>> nested;
122-
std::map<std::string, std::shared_ptr<int>> map1;
123-
map1["a"] = std::make_shared<int>(1);
124-
map1["b"] = std::make_shared<int>(2);
125-
std::map<std::string, std::shared_ptr<int>> map2;
126-
map2["c"] = std::make_shared<int>(3);
127-
map2["d"] = nullptr;
128-
nested.push_back(std::move(map1));
129-
nested.push_back(std::move(map2));
122+
EXPECT_THAT(str, ::testing::HasSubstr("Alice: 95.5"));
123+
EXPECT_THAT(str, ::testing::HasSubstr("Bob: 87"));
124+
EXPECT_THAT(str, ::testing::HasSubstr("Charlie: null"));
125+
126+
std::vector<std::map<std::string, std::shared_ptr<int>>> nested = {
127+
{{"a", std::make_shared<int>(1)}, {"b", std::make_shared<int>(2)}},
128+
{{"c", std::make_shared<int>(3)}, {"d", nullptr}},
129+
};
130130
EXPECT_EQ("[{a: 1, b: 2}, {c: 3, d: null}]", std::format("{}", nested));
131131
}
132132

0 commit comments

Comments
 (0)