From 73a51ef14fcf610c96db670342b7df10ab73a474 Mon Sep 17 00:00:00 2001 From: tbeu Date: Wed, 12 Feb 2025 07:40:50 +0100 Subject: [PATCH] Fix C++ code smells * Move "using namespace std;" from header to module file (to no longer pollute the global namespace). * Fix inconsistency of having namespace prefix in test code. * Add include guard to GildedRose.h. * Avoid std::endl. * Have consistent white-space. --- cpp/src/GildedRose.cc | 2 ++ cpp/src/GildedRose.h | 13 ++++++------- .../GildedRoseCatch2ApprovalTests.cc | 6 +++--- .../GildedRoseCatch2UnitTests.cc | 2 +- .../GildedRoseGoogletestApprovalTests.cc | 6 +++--- .../GildedRoseGoogletestUnitTests.cc | 2 +- cpp/test/cpp_texttest/GildedRoseTextTests.cc | 14 ++++++-------- 7 files changed, 22 insertions(+), 23 deletions(-) diff --git a/cpp/src/GildedRose.cc b/cpp/src/GildedRose.cc index 8df23e47..68436ace 100644 --- a/cpp/src/GildedRose.cc +++ b/cpp/src/GildedRose.cc @@ -1,5 +1,7 @@ #include "GildedRose.h" +using namespace std; + GildedRose::GildedRose(vector & items) : items(items) {} diff --git a/cpp/src/GildedRose.h b/cpp/src/GildedRose.h index 8464f87b..9af16efc 100644 --- a/cpp/src/GildedRose.h +++ b/cpp/src/GildedRose.h @@ -1,24 +1,23 @@ +#pragma once + #include #include -using namespace std; - class Item { public: - string name; + std::string name; int sellIn; int quality; - Item(string name, int sellIn, int quality) : name(name), sellIn(sellIn), quality(quality) + Item(std::string name, int sellIn, int quality) : name(name), sellIn(sellIn), quality(quality) {} }; class GildedRose { public: - vector & items; - GildedRose(vector & items); + std::vector & items; + GildedRose(std::vector & items); void updateQuality(); }; - diff --git a/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc b/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc index 6b457f3d..27c173db 100644 --- a/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc +++ b/cpp/test/cpp_catch2_approvaltest/GildedRoseCatch2ApprovalTests.cc @@ -12,12 +12,12 @@ std::ostream& operator<<(std::ostream& os, const Item& obj) TEST_CASE("GildedRoseApprovalTests", "VerifyCombinations") { - std::vector names { "Foo" }; + std::vector names { "Foo" }; std::vector sellIns { 1 }; std::vector qualities { 1 }; - auto f = [](string name, int sellIn, int quality) { - vector items = {Item(name, sellIn, quality)}; + auto f = [](std::string name, int sellIn, int quality) { + std::vector items = {Item(name, sellIn, quality)}; GildedRose app(items); app.updateQuality(); return items[0]; diff --git a/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc b/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc index a4fe144e..84f706b1 100644 --- a/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc +++ b/cpp/test/cpp_catch2_unittest/GildedRoseCatch2UnitTests.cc @@ -3,7 +3,7 @@ TEST_CASE("GildedRoseUnitTest", "Foo") { - vector items; + std::vector items; items.push_back(Item("Foo", 0, 0)); GildedRose app(items); app.updateQuality(); diff --git a/cpp/test/cpp_googletest_approvaltest/GildedRoseGoogletestApprovalTests.cc b/cpp/test/cpp_googletest_approvaltest/GildedRoseGoogletestApprovalTests.cc index 743975bb..f7d6dc3a 100644 --- a/cpp/test/cpp_googletest_approvaltest/GildedRoseGoogletestApprovalTests.cc +++ b/cpp/test/cpp_googletest_approvaltest/GildedRoseGoogletestApprovalTests.cc @@ -12,12 +12,12 @@ std::ostream& operator<<(std::ostream& os, const Item& obj) TEST(GildedRoseApprovalTests, VerifyCombinations) { - std::vector names { "Foo" }; + std::vector names { "Foo" }; std::vector sellIns { 1 }; std::vector qualities { 1 }; - auto f = [](string name, int sellIn, int quality) { - vector items = {Item(name, sellIn, quality)}; + auto f = [](std::string name, int sellIn, int quality) { + std::vector items = {Item(name, sellIn, quality)}; GildedRose app(items); app.updateQuality(); return items[0]; diff --git a/cpp/test/cpp_googletest_unittest/GildedRoseGoogletestUnitTests.cc b/cpp/test/cpp_googletest_unittest/GildedRoseGoogletestUnitTests.cc index 654b3d66..918f9dbf 100644 --- a/cpp/test/cpp_googletest_unittest/GildedRoseGoogletestUnitTests.cc +++ b/cpp/test/cpp_googletest_unittest/GildedRoseGoogletestUnitTests.cc @@ -2,7 +2,7 @@ #include "GildedRose.h" TEST(GildedRoseTest, Foo) { - vector items; + std::vector items; items.push_back(Item("Foo", 0, 0)); GildedRose app(items); app.updateQuality(); diff --git a/cpp/test/cpp_texttest/GildedRoseTextTests.cc b/cpp/test/cpp_texttest/GildedRoseTextTests.cc index 548d1d6a..bb570513 100644 --- a/cpp/test/cpp_texttest/GildedRoseTextTests.cc +++ b/cpp/test/cpp_texttest/GildedRoseTextTests.cc @@ -3,12 +3,12 @@ void print_item(Item& item) { - std::cout << item.name << ", " << item.sellIn << ", " << item.quality << std::endl; + std::cout << item.name << ", " << item.sellIn << ", " << item.quality << '\n'; } int main() { - vector items; + std::vector items; items.push_back({"+5 Dexterity Vest", 10, 20}); items.push_back({"Aged Brie", 2, 0}); @@ -22,22 +22,20 @@ int main() // this Conjured item doesn't yet work properly items.push_back({"Conjured Mana Cake", 3, 6}); - std::cout << "OMGHAI!" << std::endl; + std::cout << "OMGHAI!" << '\n'; GildedRose app(items); for (int day = 0; day <= 30; day++) { - std::cout << "-------- day " << day << " --------" << std::endl; - std::cout << "name, sellIn, quality" << std::endl; + std::cout << "-------- day " << day << " --------" << '\n'; + std::cout << "name, sellIn, quality" << '\n'; for (auto& item : items) { print_item(item); } - std::cout << std::endl; + std::cout << '\n'; app.updateQuality(); } return 0; } - -