И iTeam класс: ответ на один из нерешенных переполнение стека Кес


Я хотел бы улучшить мои навыки на понятия С++ и наткнулся на следующий вопрос на сайте StackOverflow, который был прямо спросил на сайте StackOverflow пользователя без каких-либо предварительных попыток.(к сожалению, у меня только образ, поскольку фактическое пост был неразрешимый вопрос, она была удалена с сайта самим пользователем) . Любые советы или замечания с целью оптимизации или улучшения кода и вопросов, правильность решения можно будет только приветствовать. .

#include <iostream>
#include <string>
#include <unordered_map>

/** Class which has been asked in the question. In short Iteam class will act like
    a struct for iteamList class, since I have declared iteamList class as a friend
    of Iteam class **/
class iteamList; // forward delaration of iteamList class
class Iteam
{
private:
    int m_code;
    int m_price;
    int m_quantity;
public:
    Iteam()                                     // Defualt
        :m_code(0), m_price(0), m_quantity(0)
        {}
    Iteam(const int& code, const  int& price, const int& quntity)// Parameterized
        :m_code(code), m_price(price), m_quantity(quntity)
        {}
    Iteam(const Iteam& rhs)                     // Copy
        :m_code(rhs.m_code), m_price(rhs.m_price), m_quantity(rhs.m_quantity)
        {}
    Iteam(Iteam&& rhs)                          // Move
        :m_code(rhs.m_code), m_price(rhs.m_price), m_quantity(rhs.m_quantity)
        {}
    ~Iteam()     {}

    //(i) input the data members
    inline void inputData()
    {
        std::cout<<"Iteam code :"; std::cin>>m_code;
        std::cout<<"Iteam price :"; std::cin>>m_price;
        std::cout<<"Iteam quantity :"; std::cin>>m_quantity;
    }
    // life would be easy after making this statement :) !
    friend iteamList;
    //(ii) to display the data members
    friend std::ostream& operator<<(std::ostream& output, const Iteam& obj);
};
std::ostream& operator<<(std::ostream& output, const Iteam& obj)
{
    return output<<"Iteam code = "<<obj.m_code
                <<"\t price = "<<obj.m_price
                <<"\t quantity = "<<obj.m_quantity<<"\n";
}
/******************************************************************************/
/** Implementation of the main class which is independent of the above one.
    It can access all members of above class if it has instantiated an object of it.
    I have used STL std::unordered_map to store the Items which will be easier to
    look up(with an O(1)) at the time of queries of deletion.**/

class iteamList
{
private:
    int m_totalStockValue = 0;
    std::unordered_map<int, Iteam> m_iteamMap;
private:
    // (iii) appending iteam to the list
    inline void AppendIteams(const int& userInput_n)
    {
        for(auto i=0; i<userInput_n; ++i)
        {
            Iteam obj;
            obj.inputData();
            std::cout << obj << std::endl;
            m_iteamMap.emplace(obj.m_code, obj);
        }
    }
public:
    iteamList() = default;

    iteamList(const int& userInput_n)
    {
        m_iteamMap.reserve(userInput_n);
        AppendIteams(userInput_n);
    }
    ~iteamList(){}

    // (iv) delete the iteam from the list
    void deleteIteam(const int& code)
    {
        if(m_iteamMap.find(code) != m_iteamMap.cend())
        {
            std::cout << m_iteamMap[code] << " has been deleted!\n";
            m_iteamMap.erase(m_iteamMap.find(code));
        }
        else
            std::cout <<"Invalid Iteam code!" << std::endl;
    }
    // (v) total value of the stock
    void claculateStockValue()
    {
        m_totalStockValue = 0;
        for(const auto &it:m_iteamMap)
            m_totalStockValue += (it.second.m_price * it.second.m_quantity);

        std::cout <<m_totalStockValue<< std::endl;
    }
};
/**********************************************************************************/
int main()
{
    int number;
    std::cout <<"Enter total number of iteams : ";
    std::cin>>number;

    iteamList obj(number);

    std::cout <<"Total Value of stock = ";
    obj.claculateStockValue();

    int code;
    std::cout <<"Enter code to delete the iteam : ";
    std::cin>>code;
    obj.deleteIteam(code);

    std::cout <<"Total Value of stock = ";
    obj.claculateStockValue();

    return 0;
}


187
5
задан 23 марта 2018 в 09:03 Источник Поделиться
Комментарии
1 ответ


  1. Во-первых: пожалуйста, исправьте орфографию. Функции, которые вы пытаетесь внедрить вроде бы о предметах, так что ваши классы должны называться Item и ItemListсоответственно. Кроме того, есть по крайней мере одно место в коде, где ты сделал ошибку в слове "количество", как "количество". claculateStockValue должно быть calculateStockValue. Я знаю, что это трудно для не-носителя языка, чтобы сделать такие вещи (я не родной для себя, поэтому я понимаю вашу борьбу). Однако, чтобы сделать ваш код чистым и легким для чтения, важно иметь определенный уровень чистоты на протяжении именования и комментирования.

  2. Говоря о чистоте имя, сохранить свою капитализацию последовательны. Почему iteamList начать со строчной я пока Iteam начинается с прописной? Решить по одной схеме именования и придерживайтесь его. То же самое относится к именам метод.

  3. Вместо определения конструктора по умолчанию для Iteam что не делает ничего особенного, назначить по умолчанию член инициализаторы и = default этот конструктор (как и в другом классе).

  4. Вам не нужно определить копировать и перемещать конструктор в Iteam. Компилятор достаточно хорошо, чтобы генерировать их автоматически в большинстве случаев. Вам нужно только определить их самостоятельно, если ваш класс делает некоторые необычные управляющий ресурс, или что-то подобное. То же верно и для деструктора.

  5. Не берут int как константная ссылка. Взять его, а не по значению. Я не знаю ни одного архитектуры там, где и int не могли передаваться эффективно в реестр (или в стеке).

  6. Вам не нужно объявлять вперед iteamList. Вместо этого вы можете просто включить friend iteamList; в friend class iteamList;.

  7. На самом деле, это случай friend-декларации злоупотребления. Вместо того, чтобы делать iteamList друг Iteam, Iteam следует определить фактическое общедоступный интерфейс (т. е. геттеры и сеттеры). Как сейчас, весь Iteam класс является несколько бесполезной.

  8. Не использовать inline если вы точно знаете, что вы делаете. В большинстве случаев, компилятор все равно игнорируют.

  9. Не определить inputData как член Iteamнарушает принцип единой ответственности. Чтение и парсинг данных не является частью ответственности Iteam.

  10. iteamList управляет его содержание с помощью... std::unordered_map?! Надеюсь, вы понимаете, что это вопиющее несоответствие между именем и функциональностью: когда я вижу класс, который имеет "список" как часть его имени, я жду его, чтобы использовать какой-то список, чтобы сохранить его содержимое.

  11. Это, как говорится, std::unordered_map не кажется, очень хорошо подойдет здесь. Действительно, хотел бы предложить себя очень хорошо, если вы хотите сохранить за O(1) удаление и вставка собственность. Кроме того, идентификатор уже является частью Iteamы; она является избыточной, чтобы добавить его в качестве ключа для карты. Если вы хотите сохранить карту, необходимо извлечь ключ из Iteam класс.

  12. Опять же, такие методы, как AppendIteams нарушает принцип единой ответственности и должны быть извлечены из своего класса. Ответственность iteamList управление перечнем элементов; чтение и создание элементов является обязанностью другой кусок кода.

  13. Кроме того, не выводить отладочные сообщения из своего класса напрямую. Это, опять же, нарушает ПСП.

  14. Держать интервал вокруг операторов последовательной, особенно вокруг << и >>. В целом, считается хорошей практикой оставить пробел перед и после всех операторов.

  15. Как с Iteam, iteamList сильно не хватает публичного интерфейса. Пожалуйста, добавьте методы getter и setter в обмен на все эти ИО-методов.

  16. Не держать m_totalStockValue вокруг члена. Сделать его локальным для claculateStockValue и вместо того, чтобы вернуть его.

8
ответ дан 23 марта 2018 в 10:03 Источник Поделиться