code cleanup

CMakeLists.txt

@@ -10,7 +10,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 if(MSVC)
   add_compile_options(/W4 /WX)
 else()
-  add_compile_options(-Wall -Wpedantic)
+  add_compile_options(-Wall -Wextra -pedantic -Woverloaded-virtual -Wredundant-decls -Wshadow)
 endif()
 
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)

src/core/article.cpp

@@ -1,6 +1,6 @@
 #include "article.h"
 
-Article::Article() : Entity() {}
+//Article::Article() : Entity() {}
 
 // Article::Article(std::shared_ptr<Seller> sellerPtr) : Entity() { sellerPtr_ = sellerPtr; }
 
@@ -20,8 +20,6 @@ std::string Article::getDescription() { return description_; }
 
 Seller* Article::getSeller() { return sellerPtr_; }
 
-int Article::getPrice() { return price_; }
+int Article::getPrice() const { return price_; }
 
-int Article::getArticleNo() {
-    return articleNo_;
-}
+int Article::getArticleNo() { return articleNo_; }

src/core/article.h

@@ -14,20 +14,20 @@ class Sale;
 class Article : public Entity
 {
   public:
-    Article();
-    //Article(std::shared_ptr<Seller> sellerPtr);
+    Article() = default;
+    //virtual ~Article() = default;
+
     void setArticleNo(int articleNo);
     void setPrice(int price);
     void setDescription(const std::string& description);
     bool isSold();
     void setSale(Sale* salePtr);
-    //void setSeller(std::shared_ptr<Seller> sellerPtr);
     void setSeller(Seller* sellerPtr);
 
     int getArticleNo();
     std::string getDescription();
     Seller* getSeller();
-    int getPrice();
+    int getPrice() const;
 
   private:
     Seller* sellerPtr_{};

src/core/entity.cpp

@@ -5,7 +5,7 @@
 #include <boost/uuid/uuid_generators.hpp>
 #include <boost/uuid/uuid_io.hpp>
 
-Entity::~Entity() {}
+Entity::~Entity() = default;
 
 void Entity::createUuid()
 {
@@ -19,7 +19,7 @@ void Entity::setUuidFromString(const std::string& uuidString)
     uuid_ = generator(uuidString);
 }
 
-inline Entity::State Entity::getState() const
+Entity::State Entity::getState() const
 {
     return state_;
 }

src/core/entity.h

@@ -11,6 +11,7 @@ class Entity
   public:
     enum class State { NEW, UPDATE, DELETE, OK };
 
+    //Entity() = default;
     virtual ~Entity() = 0;
 
     void createUuid();

src/core/marketplace.cpp

@@ -22,15 +22,13 @@ void Marketplace::loadFromDb()
     db.loadSellers(sellers_);
 }
 
-std::vector<std::unique_ptr<Seller>>& Marketplace::getSellers() { return sellers_; }
+SellersVec& Marketplace::getSellers() { return sellers_; }
 
 int Marketplace::getNextSellerNo()
 {
     auto iter = std::max_element(
         sellers_.begin(), sellers_.end(),
-        [](const std::unique_ptr<Seller>& a, const std::unique_ptr<Seller>& b) -> bool {
-            return a->getSellerNo() < b->getSellerNo();
-        });
+        [](const auto& a, const auto& b) -> bool { return a->getSellerNo() < b->getSellerNo(); });
     if (iter == sellers_.end())
         return 1;
     return (*iter)->getSellerNo() + 1;
@@ -43,7 +41,4 @@ int Marketplace::getNumSellersDelete()
     return count;
 }
 
-void Marketplace::sortSellers()
-{
-    std::sort(sellers_.begin(), sellers_.end());
-}
+void Marketplace::sortSellers() { std::sort(sellers_.begin(), sellers_.end()); }

src/core/marketplace.h

@@ -7,20 +7,29 @@
 
 #include <vector>
 
+namespace
+{
+using SellersVec = std::vector<std::unique_ptr<Seller>>;
+using SalesVec = std::vector<std::unique_ptr<Sale>>;
+} // namespace
+
+struct Basket {
+};
+
 class Marketplace
 {
   public:
     Marketplace();
     void storeToDb(bool onlyDelete = false);
     void loadFromDb();
-    std::vector<std::unique_ptr<Seller>>& getSellers();
+    SellersVec& getSellers();
     int getNextSellerNo();
     int getNumSellersDelete();
     void sortSellers();
 
   private:
-    std::vector<std::unique_ptr<Seller>> sellers_;
-    std::vector<std::unique_ptr<Sale>> sales_;
+    SellersVec sellers_;
+    SalesVec sales_;
 };
 
 #endif

src/core/sale.cpp

@@ -2,26 +2,20 @@
 
 #include <numeric>
 
-void Sale::addArticle(std::shared_ptr<Article> articlePtr)
+void Sale::addArticle(Article* articlePtr)
 {
     articlePtr->setSale(this);
     articles_.push_back(articlePtr);
 }
 
-std::vector<Article*> Sale::getArticles()
-{
-    std::vector<Article*> articles(articles_.size());
-    for (const auto& article : articles_) {
-        articles.push_back(article.get());
-    }
-
-    return articles;
-}
+std::vector<Article*>& Sale::getArticles() { return articles_; }
 
 void Sale::removeArticle(const Article* articlePtr)
 {
-    auto it = std::find_if(articles_.begin(), articles_.end(),
-                           [&articlePtr](auto art) { return art.get() == articlePtr; });
+    /* auto it = std::find_if(articles_.begin(), articles_.end(),
+                               [&articlePtr](auto art) { return art.get() == articlePtr; }); */
+    auto it = std::find(articles_.begin(), articles_.end(), articlePtr);
+
     if (it != articles_.end()) {
         (*it)->setSale(nullptr);
         (*it)->setState(
@@ -33,7 +27,7 @@ void Sale::removeArticle(const Article* articlePtr)
 int Sale::sumInCents()
 {
     int sum = std::accumulate(articles_.begin(), articles_.end(), 0,
-                              [](int a, std::shared_ptr<Article> b) { return a + b->getPrice(); });
+                              [](int a, const Article* b) { return a + b->getPrice(); });
     return sum;
 }
 

src/core/sale.h

@@ -9,13 +9,15 @@
 
 class Article;
 
+using ArticlesVec = std::vector<Article*>;
+
 class Sale : public Entity
 {
   public:
-    void addArticle(std::shared_ptr<Article> articlePtr);
+    void addArticle(Article* articlePtr);
     void setTimestamp(const std::string& timestamp);
 
-    std::vector<Article*> getArticles();
+    ArticlesVec& getArticles();
     std::string getTimestamp();
     int sumInCents();
 
@@ -24,7 +26,7 @@ class Sale : public Entity
   private:
     std::string timestamp_{
         boost::posix_time::to_iso_extended_string(boost::posix_time::second_clock::local_time())};
-    std::vector<std::shared_ptr<Article>> articles_{};
+    ArticlesVec articles_{};
 };
 
 #endif

src/core/seller.cpp

@@ -1,10 +1,7 @@
 #include "seller.h"
 
-Seller::Seller() : Entity() {}
-
 Seller::Seller(const std::string& firstName, const std::string& lastName, int sellerNo,
-               int numArticlesOffered)
-    : Entity()
+               int numArticlesOffered) : Entity()
 {
     firstName_ = firstName;
     lastName_ = lastName;
@@ -20,10 +17,10 @@ void Seller::setLastName(const std::string& lastName) { lastName_ = lastName; }
 
 void Seller::setNumArticlesOffered(int number) { numArticlesOffered_ = number; }
 
-void Seller::addArticle(std::shared_ptr<Article> article)
+void Seller::addArticle(std::unique_ptr<Article> article)
 {
     article->setSeller(this);
-    articles_.push_back(article);
+    articles_.push_back(std::move(article));
 }
 
 std::string Seller::getFirstName() const { return firstName_; }
@@ -35,7 +32,7 @@ int Seller::getSellerNo() const { return sellerNo_; }
 std::vector<Article*> Seller::getArticles(bool onlySold) const
 {
     std::vector<Article*> articles;
-    for (const auto article : articles_) {
+    for (const auto& article : articles_) {
         if (onlySold && article->isSold()) {
             articles.push_back(article.get());
         } else if (!onlySold) {
@@ -52,7 +49,7 @@ int Seller::numArticlesOffered() const { return numArticlesOffered_; }
 void Seller::cleanupArticles()
 {
     articles_.erase(std::remove_if(articles_.begin(), articles_.end(),
-                                   [](const std::shared_ptr<Article>& article) {
+                                   [](const auto& article) {
                                        return article->getState() == Article::State::DELETE;
                                    }),
                     articles_.end());

src/core/seller.h

@@ -13,7 +13,8 @@ class Article;
 class Seller : public Entity
 {
   public:
-    Seller();
+    Seller() = default;
+    //virtual ~Seller() = default;
     Seller(const std::string& firstName, const std::string& lastName, int sellerNo = 0,
            int numArticlesOffered = 0);
 
@@ -21,7 +22,7 @@ class Seller : public Entity
     void setFirstName(const std::string& firstName);
     void setLastName(const std::string& lastName);
     void setNumArticlesOffered(int number);
-    void addArticle(std::shared_ptr<Article> article);
+    void addArticle(std::unique_ptr<Article> article);
     void cleanupArticles();
 
     std::string getFirstName() const;
@@ -40,7 +41,7 @@ class Seller : public Entity
     int numArticlesOffered_{};
     std::string firstName_{};
     std::string lastName_{};
-    std::vector<std::shared_ptr<Article>> articles_{};
+    std::vector<std::unique_ptr<Article>> articles_{};
 };
 
 #endif

src/gui/sellermodel.cpp

@@ -9,12 +9,12 @@ SellerModel::SellerModel(Marketplace* market, QObject* parent)
 {
 }
 
-int SellerModel::rowCount(const QModelIndex& parent) const
+int SellerModel::rowCount([[maybe_unused]] const QModelIndex& parent) const
 {
     return marketplace_->getSellers().size();
 }
 
-int SellerModel::columnCount(const QModelIndex& parent) const { return 5; }
+int SellerModel::columnCount([[maybe_unused]] const QModelIndex& parent) const { return 5; }
 
 QVariant SellerModel::data(const QModelIndex& index, int role) const
 {
@@ -92,8 +92,8 @@ bool SellerModel::setData(const QModelIndex& index, const QVariant& value, int r
             return false;
         auto iter =
             std::find_if(marketplace_->getSellers().begin(), marketplace_->getSellers().end(),
-                         [&value](const std::unique_ptr<Seller>& seller) {
-                             return value.toInt() == seller->getSellerNo();
+                         [&value](const std::unique_ptr<Seller>& s) {
+                             return value.toInt() == s->getSellerNo();
                          });
         if (iter != marketplace_->getSellers().end()) {
             emit duplicateSellerNo(

src/gui/sellermodel.h

@@ -16,7 +16,7 @@ class SellerModel : public QAbstractTableModel
     virtual QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override;
     virtual QVariant headerData(int section, Qt::Orientation orientation, int role) const override;
     virtual Qt::ItemFlags flags(const QModelIndex& index) const override;
-    virtual bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole);
+    virtual bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override;
     virtual bool insertRows(int row, int count, const QModelIndex& parent = QModelIndex()) override;
     virtual bool removeRows(int row, int count, const QModelIndex& parent = QModelIndex()) override;
 

test/test_article.cpp

@@ -15,7 +15,7 @@ BOOST_AUTO_TEST_CASE(check_is_sold)
     Article article{};
     BOOST_TEST(article.isSold() == false);
 
-    auto salePtr = std::make_shared<Sale>();
-    article.setSale(salePtr.get());
+    auto salePtr = std::make_unique<Sale>();
+    salePtr->addArticle(&article);
     BOOST_TEST(article.isSold() == true);
 }

test/test_database.cpp

@@ -30,11 +30,11 @@ BOOST_AUTO_TEST_CASE(store_sellers_succ)
     auto b = std::make_unique<Seller>("Max", "Mustermann");
     b->createUuid();
     b->setSellerNo(2);
-    auto c = std::make_shared<Article>();
+    auto c = std::make_unique<Article>();
     c->createUuid();
     c->setPrice(500);
     c->setDescription("Test");
-    b->addArticle(c);
+    b->addArticle(std::move(c));
     BOOST_TEST(a->getUuid() != b->getUuid());
     sellers.push_back(std::move(a));
     sellers.push_back(std::move(b));

test/test_sale.cpp

@@ -12,15 +12,15 @@ BOOST_AUTO_TEST_CASE(articles_sum)
     const int NUM_ARTICLES = 30;
 
     Seller seller("Max", "Mustermann", 1, NUM_ARTICLES);
-    std::vector<std::shared_ptr<Article>> articles{};
+    ArticlesVec articles{};
     Sale sale{};
 
     for(int i = 0; i < NUM_ARTICLES; ++i)
     {
-        auto art = std::make_shared<Article>();
+        auto art = std::make_unique<Article>();
         art->setPrice((i+1) * 10);
-        articles.push_back(art);
-        seller.addArticle(art);
+        articles.push_back(art.get());
+        seller.addArticle(std::move(art));
     }
 
     for(int i = 0; i < 10; ++i)
@@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(remove_article) {
     Sale sale{};
 
     BOOST_TEST(art->isSold() == false);
-    sale.addArticle(art);
+    sale.addArticle(art.get());
     BOOST_TEST(art->isSold() == true);
     sale.removeArticle(art.get());
     BOOST_TEST(art->isSold() == false);

test/test_seller.cpp

@@ -30,10 +30,11 @@ BOOST_AUTO_TEST_CASE(create_many)
 }
 
 BOOST_AUTO_TEST_CASE(with_article) {
-    Seller seller("Max", "Mustermann");
-    auto article = std::make_shared<Article>();
+    Seller seller{};
+    auto article = std::make_unique<Article>();
     article->setDescription("Test article");
-    seller.addArticle(article);
+    seller.addArticle(std::move(article));
+    BOOST_TEST((article == nullptr));
     BOOST_TEST(seller.getArticles(false).at(0)->getDescription() == "Test article");
     BOOST_TEST(seller.numArticlesSold() == 0);
 }