Реализация собаководства


Мне нужен анализ кода на следующий вопрос:

На C++ объектно-ориентированное проектирование, реализация Kennel так что:

  • Метод AddCat() добавляет Cat к Kennel, предоставляя своим именем.
  • Метод AddDog() добавляет Dog к Kennel, предоставляя своим именем.
  • Метод RollCall() печать Animal'ы имя и звук stdout:
    • CatС идентифицировать себя путем печати "Meow" для stdout.
    • DogС идентифицировать себя путем печати "Woof" для stdout.

Питомник .ч

    #pragma once
    #ifndef KENNEL_H
    #define KENNEL_H

    #include <string>
    #include <iostream>
    #include <vector>

    class Kennel
    {

    public:
        Kennel() { };
        virtual ~Kennel();
        void AddCat(const std::string & name);
        void AddDog(const std::string & name);
        void RollCall();
        virtual void makeSound(std::string name) { }

    private:
        std::vector <Kennel*> KennelList;
    protected:
        std::string name;

    };

    //Dog inherits Kennel
    class Dog :public Kennel
    {
    public:
        Dog(std::string dogName)
        {
            name = dogName;
        }
        ~Dog() { };
        void makeSound(std::string name)
        {
            std::cout << name << " says Woof" << std::endl;
        };
    };

    //Cat inherits Kennel
    class Cat :public Kennel
    {
    public:
        Cat(std::string catName)
        {
            name = catName;
        }
        ~Cat() { };
        void makeSound(std::string name)
        {
            std::cout << name << " says Meow" << std::endl;
        };
    };


    #endif

Kennel.cpp

    #include "Kennel.h"


    Kennel::~Kennel()
    {
        for (auto i : KennelList)
        {
            delete i;
        }
    }

    void Kennel::AddCat(const std::string & name)
    {
        KennelList.push_back(new Cat(name));
    }

    void Kennel::AddDog(const std::string & name)
    {
        KennelList.push_back(new Dog(name));
    }

    void Kennel::RollCall()
    {
        for (unsigned int i = 0; i < KennelList.size(); ++i)
        {
            KennelList[i]->makeSound(KennelList[i]->name);
        }
    }

главная

#include "Kennel.h"
    int main()
    {
    Kennel kennel;

    kennel.AddCat("Garfield");
    kennel.AddDog("Odie");
    kennel.AddDog("Pluto");
    kennel.AddCat("Felix");
    kennel.AddCat("Sylvester");
    kennel.AddCat("Scratchy");
    kennel.AddDog("Scooby Doo");
    kennel.AddCat("Puss in Boots");
    kennel.AddDog("Goofy");
    kennel.AddDog("Old Yeller");

    kennel.RollCall();
}


1670
8
задан 20 февраля 2018 в 02:02 Источник Поделиться
Комментарии
5 ответов

Мне нравится, как вы сделали Kennel класс виртуальная база для ваших животных. В других отзывах есть смысл в том, что это может стать запутанным, но для такого простого проекта, Я думаю, это достаточно эффективно. Это устраняет необходимость для чисто виртуальной Animal класса, который является своего рода аккуратный. Заметь, я весьма скептически относился к "объектно-ориентированный" подход, как это обычно практикуется. Я могу пересчитать по пальцам, сколько раз я могу использовать наследование, чтобы реально упростить вещи.

Вы также не используете using namespace std. Фантастика!

Именования

Ваши имена переменных в порядке по большей части. Но обратите внимание, что AddCat и AddDog использовать name в качестве входного аргумента, и объект имеет name переменная-член также. Таким образом, в этих функциях переменной-члена не видно. Вы должны попробовать, чтобы избежать дублирования имен такой. Простым решением является, чтобы иметь переменные-члены последовательно назван m_name или name_. Это делает его очевидным, что они, и это также предотвращает столкновения именем.

Методы реализованы на основе осуществления требований имеют различные наименования стиль, чем makeSound метод, который вы создали. У вас также есть две переменные-члены, начинается с заглавной буквы, а другие со строчной буквы. Старайтесь быть последовательны в наименовании!

Управление памятью

Деструктор Kennel перебирает KennelList и разрушает всю выделенную память. Судя по всему вы не утечка памяти. Но в идеале вы не хотите иметь дело с освобождением. Почему бы не дать понять компилятору, что для освобождения?

std::vector<std::unique_ptr<Kennel>> KennelList;

Если вы используете вектор std::unique_ptr элементы, вам не нужно беспокоиться о delete. Когда вы вставляете указатель в KennelListон будет с этого момента ухаживать не только указатель, но и указал-для сведения. Вы могли бы сделать нечто подобное, чтобы поставить указатели в векторе:

KennelList.emplace_back(new Cat(name));

Входных аргументов

Для AddCat способ вы берете ссылку на строку в качестве входного аргумента. Это отличный (хотя я предпочитаю писать std::string const& name а не const std::string & nameэто предотвращает некоторые недоразумения между мной и компилятор...). Однако, для всех других методов вы берете std::string по стоимости (Dog и Cat конструкторы, makeSound). Нет необходимости, чтобы сделать эти копии.

Хоть одна альтернатива для конструктора может быть

Dog(std::string dogName)
{
name = std::move(dogName);
}

Здесь, вместо того, чтобы dogName по ссылке, вы принимаете его значение, сделав копию. Эта копия тогда вы двигаетесь в переменной-члена. Преимущество изготовления копии на вход функции, которая при вызове функции,

Dog("Kipper");

временное std::string сделан. Компилятор не будет создавать копию этого временного передать Dog конструктор, но он будет непосредственно использовать временные. Таким образом, std::string строится только один раз и хранятся непосредственно в ваш новый объект.

Отметим также, что makeSound не нужен входной аргумент вообще. Объект должен знать свое имя!

Петли

В деструкторе можно использовать современные петли:

for (auto i : KennelList)

Но в RollCall вы пишете

for (unsigned int i = 0; i < KennelList.size(); ++i)

Опять же, консистенция хорошая. Кроме того, первая форма петли лучше, потому что он менее многословен, и, таким образом, гораздо легче читать.

std::endl

std::endl не только печатает новую строку, он также очищает поток. Гораздо эффективнее позволить системе самой решить, когда для очистки.

std::cout << name << " says Woof" << std::endl;

производит те же выходные данные,

std::cout << name << " says Woof\n";

Наследование

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

class Kennel {
public:
void AddCat(std::string name) {
kennelList_.emplace_back(Species::Cat, std::move(name));
}
void AddDog(std::string const& name) {
kennelList_.emplace_back(Species::Dog, std::move(name));
}
void RollCall() {
for (auto animal : kennelList_) {
std::cout << animal.name;
switch (animal.species) {
case Species::Dog:
std::cout << " says Woof\n";
break;
case Species::Cat:
std::cout << " says Meow\n";
break;
}
}
}
private:
enum class Species { Dog, Cat };
struct Animal {
Species species;
std::string name;
Animal(Species s, std::string n)
: species(s), name(std::move(n)) {}
};
std::vector <Animal> kennelList_;
};

Кроме простой код, он также является более эффективным. Не в этом дело вообще в это приложение, но мне нравится думать об эффективности. Не стесняйтесь, чтобы пропустить остальную часть этого раздела, вы только начинаете, но, возможно, вы найдете это интересным.

Одно из важнейших понятий современного оборудования является локальность данных. Данные, которые близко друг к другу быстрее процесс, потому что он играет лучше с кэшем. Другой важный понятие предсказания ветвлений. Современные аппаратные пытается предсказать, чем закончится условный оператор, и начинается выполнение кода, даже если он не сделал вычислений, что состояние зависит от. Если она не сможет правильно предсказать, вещи выбрасывают в мусор и он начинает процесс правильное направление. Если он может предсказать, Ну, if заявления не откладывайте все, что много. В противном случае, это хорошо, чтобы предотвратить ifС.

Обратите внимание, что происходит при обработке элементов std::vector<Kennel*>: указатели сами являются смежными в памяти, это легко, чтобы получить их. Но они указывают на вещи, выделенные самостоятельно, и поэтому, возможно, не непрерывно. В RollCall функция должна принести эти объекты (кэш неэффективной здесь), сделать их таблицы виртуальных функций указатель, найти адрес в makeSound метод, а затем вызвать метод (предсказание ветвлений не применимо, потому что процессор буквально ждет указатель, который указывает на код, который будет выполнен).

Сравните это с тем, что происходит в простой код выше: Animal объектами являются все непрерывные в памяти (потому что мы держать вещи прямо). В RollCall метод петли над ними, и одно из двух отделений код выполняется в зависимости от значения в нем. Кэш и предсказания ветвлений может делать свое дело здесь.

Хотя, поскольку то, что делается внутри цикла писать stdoutвсе вышесказанное является способ быстрый по сравнению, и эффективность имеет большого значения здесь.

0
ответ дан 21 февраля 2018 в 10:02 Источник Поделиться

Думаю, что фраза : public значит "это".

Собака это не питомник, ни кошка, так Dog и Cat не должны наследовать от Kennel. Вам нужен класс Animal. Кошки и собаки-это животные, поэтому Cat и Dog должен быть производным от Animal. Животные, содержащиеся в конуре.

10
ответ дан 20 февраля 2018 в 02:02 Источник Поделиться

Далее Джайв Дадсон упомянул.

Вы можете уменьшить ваши методы в питомнике

void AddAnimal(Animal *animal);

Это может быть вызван

kennel.AddAnimal(new Cat("Garfield")); // or new Dog(), as appropriate

Ваша перекличка остается неизменным, потому что ваш Animal класс имеет абстрактный метод virtual void makeSound(std::string name) { }. И это Animal класс, не Kennel класс содержит имя (Animal есть name)

protected:
std::string name;

С помощью правильного абстрагирования делает добавление других животных (например, Horse, Snake) легко и логично сделать. Используя ваш текущий код, добавление Horse значит писать Horse класса и затем AddHorse метод. С абстракцией у меня есть предположение, все, что вам сделать, это написать Horse класс (class Horse :public Animal) и Kennel класс просто работает как и раньше.

10
ответ дан 20 февраля 2018 в 06:02 Источник Поделиться

Дополняя мой предыдущий ответ (а не редактировать ее и устраняет замечания до сих пор). После обмена комментариями между мной и Крис Луенго, я думал, что есть некоторые дополнительные сведения, которые будут полезны в этом обзоре кода.

Ключевой момент сделал Крис, чистый подход к ООП не для такого простого примера целесообразно. В реальном мире я бы во многом согласен, хотя сказать Dog могут быть унаследованы от Kennel растяжка ремонтопригодность немного. Я предлагаю несколько пересмотрен ниже код на том основании, что:


  • Это часть закончил упражнения

  • Базовый код будет использоваться в дальнейшем для расширения основы ООП

  • Чистый ООП подход.

Отличные очки Крис об эффективности кода должны быть рассмотрены, если пытаешься сделать это в реальном мире.

class Kennel
{
public:
Kennel() { };
~Kennel(){
for (auto i : KennelList) {
delete i;
}
};
void ReceiveAnimal(Mammal * newAnimal){
KennelList.push_back(newAnimal);
};
void RollCall(){
for (unsigned int i = 0; i < KennelList.size(); ++i){
KennelList[i]->makeSound();
}
};
private:
std::vector <Mammal*> KennelList;
protected:
};

class Mammal
{
public:
Mammal(std::string newName){
name = newName;
}
~Mammal() { };
void makeSound()
{
std::cout << name << " says " + noise << std::endl;
};
private:
protected:
std::string name;
std::string noise;
};

//Dog inherits Mammal
class Dog :public Mammal
{
public:
Dog(std::string dogName): Mammal(dogName)
{
noise = "Woof";
name = dogName; // I don't know the language well enough, I suspect this line is not required.
}
//~Dog() { };
};
//Cat inherits Mammal
class Cat :public Mammal
{
public:
Cat(std::string catName): Mammal(catName)
{
noise = "Meow";
name = catName;
}
//~Cat() { };
};

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

Главное теперь будет выглядеть так:
тап_п()
{
Питомник собаководства;

kennel.ReceiveAnimal(new Cat("Garfield"));
kennel.ReceiveAnimal(new Dog("Odie"));
kennel.ReceiveAnimal(new Dog("Pluto"));
kennel.ReceiveAnimal(new Cat("Felix"));
kennel.ReceiveAnimal(new Cat("Sylvester"));
kennel.ReceiveAnimal(new Cat("Scratchy"));
kennel.ReceiveAnimal(new Dog("Scooby Doo"));
kennel.ReceiveAnimal(new Cat("Puss in Boots"));
kennel.ReceiveAnimal(new Dog("Goofy"));
kennel.ReceiveAnimal(new Dog("Old Yeller"));

kennel.RollCall();

}

Преимущество такого подхода заключается в том, что если у вас уже есть животное, вы можете Новый просто пропускать его через kennel.ReceiveAnimal(myExistingAnimal);

Я сделал это расширенный ответ на ООП и ремонтопригодность код, как если бы это был большие усилия (опять-таки, говорит Крис об уровне усилий для этого простого примера следует рассмотреть).

Если в питомнике решили взять новые животные (например, Лиса), а затем просто добавить новый класс (который может быть как простой, как следующий код):

class Fox :public Mammal
{
public:
Fox(std::string foxName): Mammal(foxName)
{
noise = "Ha Ha Ha! Boom! Boom!";
} // see my previous notes about inexperience with this language and assuming name will be handled by superclass.
};

Млекопитающие рожают. Используя в полном основ ООП, вы можете изменить суперкласс новый метод, который означает, что подклассы будут иметь новые функциональные возможности. Я не знаю, из-за моей неопытности, как сдержать новый способ, чтобы убедиться, что он возвращает новый экземпляр подкласса, а не суперкласса.

class Mammal
{
public:
Mammal(std::string newName){
name = newName;
}
~Mammal() { };
void makeSound()
{
std::cout << name << " says " + noise << std::endl;
};
Mammal giveBirth(std::string newName) {
return new Mammal(newName); // pardon my ignorance here but you get the gist
};

private:
protected:
std::string name;
std::string noise;
};

0
ответ дан 24 февраля 2018 в 07:02 Источник Поделиться

 #pragma once
#ifndef KENNEL_H
#define KENNEL_H

Использовать только один из этих, желательно #ПРАГМА раз. Нет необходимости в #ifndef ... #за endif.

0
ответ дан 27 февраля 2018 в 09:02 Источник Поделиться