Système de suivi des événements

Question

J'apprends la programmation C ++ et je viens d'apprendre la POO de base et j'ai décidé de créer un projet simple pour tester ma compréhension et mettre en pratique ce que j'ai appris. L'idée que j'ai eue est un système de suivi des événements dans lequel vous ajoutez des événements dans un calendrier, puis vous affichez tous vos événements. J'ai 2 classes:, Eventoù vos événements sont créés, et Calendar, qui contient un vecteur de tous les événements. Pourriez-vous s'il vous plaît revoir mon code en indiquant quelles sont les façons les plus efficaces de faire les choses et les meilleures pratiques à suivre?

---

Main.cpp

#include "Calendar.h"

int main() {
    Calendar calendar {};

    calendar.add_event("Exam", "urgent", "10/12/2020", "10:30");
    calendar.add_event("Meeting", "non-urgent", "08/11/2020", ("12:20"));

    calendar.display_events();
}

Event.h

#include <string>

class Event {
private:
    std::string event_type;
    std::string event_priority;
    std::string event_date;
    std::string event_time;

public:
    Event(std::string eventType, std::string eventPriority, std::string eventDate,
          std::string eventTime);

    bool display_event() const;

    ~Event();
};

Event.cpp

#include "Event.h"
#include <iostream>
#include <utility>

Event::Event(std::string eventType, std::string eventPriority, std::string eventDate,
             std::string eventTime) : event_type(std::move(eventType)), event_priority(std::move(eventPriority)),
                                             event_date(std::move(eventDate)), event_time(std::move(eventTime)) {
}

bool Event::display_event() const {
    std::cout << "You have " << event_type << " on " << event_date << " at " << event_time << " it's " << event_priority << "\n";
    return true;
}


Event::~Event() = default;

Calendar.h

#include "Event.h"
#include <vector>

class Calendar {
private:
    std::vector<Event> calendar;

public:
    bool display_events() const;

    bool add_event(std::string event_type, std::string event_priority, std::string event_date, std::string event_time);

    const std::vector<Event> &getCalendar() const;

    bool is_event_valid(const std::string& event_date, const std::string& event_time);

    ~Calendar();
};

Calendar.cpp

#include "Calendar.h"
#include <iostream>
#include <utility>

const std::vector<Event> &Calendar::getCalendar() const {
    return calendar;
}

bool Calendar::display_events() const {
    if (!getCalendar().empty()) {
        for (const auto &event : calendar) {
            event.display_event();
        }
        return true;
    } else {
        std::cout << "Your calendar is empty \n";
        return false;
    }
}

bool Calendar::add_event(std::string event_type, std::string event_priority, std::string event_date,
                         std::string event_time) {

    if (is_event_valid(event_date, event_time))
    {
        Event event {std::move(event_type), std::move(event_priority), std::move(event_date), std::move(event_time)};
        calendar.push_back(event);
        return true;
    } else {
        std::cout << "Event is not valid\n";
        return false;
    }
}

bool Calendar::is_event_valid(const std::string& event_date, const std::string& event_time) {
    int day{}, month{}, year{}, hours{}, minutes{};

    day = std::stoi(event_date.substr(0,2));
    month = std::stoi(event_date.substr(3, 2));
    year = std::stoi(event_date.substr(6, 4));
    hours = std::stoi(event_time.substr(0, 2));
    minutes = std::stoi(event_time.substr(3, 2));

    bool is_date_valid = (day > 0 && day <= 24) && (month > 0 && month <= 12) && (year >= 2020 && year <= 3030);
    bool is_time_valid = (hours >= 0 && hours <= 24) && (minutes >= 0 && minutes <= 60);

    if (is_date_valid && is_time_valid) {
        return true;
    } else {
        std::cout << "The event's time or date is not valid\n";
        return false;
    }
}

Calendar::~Calendar() = default;

Je pense également à ajouter une fonctionnalité permettant de trier les événements par date.

Answer 1

Observations générales

Bienvenue sur le site de révision de code. Belle question de départ, très bonne pour un programmeur C ++ débutant et un nouveau membre de la communauté de révision de code.

Les fonctions suivent le principe de responsabilité unique (SRP) qui est excellent. Les classes suivent également le SRP qui est également très bon. Vous ne faites pas une erreur de débutant assez courante en utilisant la using namespace std;déclaration. Bonne utilisation de constdans de nombreuses fonctions.

Le principe de responsabilité unique stipule:

que chaque module, classe ou fonction devrait avoir la responsabilité d'une seule partie de la fonctionnalité fournie par le logiciel, et que cette responsabilité devrait être entièrement encapsulée par ce module, cette classe ou cette fonction.

Le SRP est le S dans les principes de programmation SOLID ci-dessous.

La conception orientée objet a besoin de quelques travaux, par exemple la Eventclasse devrait avoir une is_valid()méthode pour laisser chaque événement se valider, ce serait utile lors de la création d'un nouvel événement. La Calendarclasse peut utiliser cette méthode et n'a pas besoin de connaître les membres privés de la classe d'événements. Le fait d'inclure l'accès aux membres privés de la Eventclasse dans la Calendarclasse empêche qu'il s'agisse d'une conception orientée objet SOLID .

Dans la programmation informatique orientée objet, SOLID est un acronyme mnémotechnique désignant cinq principes de conception destinés à rendre les conceptions logicielles plus compréhensibles, flexibles et maintenables.

Inclure les gardes

En C ++ ainsi que dans le langage de programmation C, le mécanisme d'importation de code #include FILEcopie en fait le code dans un fichier temporaire généré par le compilateur. Contrairement à certains autres langages modernes, C ++ (et C) inclura un fichier plusieurs fois. Pour éviter cela, les programmeurs utilisent des gardes d'inclusion qui peuvent avoir 2 formes:

1. la forme la plus portable consiste à incorporer le code dans une paire d'instructions pré-processeur

   #ifndef SYMBOL
   #define SYMBOL
   // Tous les autres codes nécessaires
   #endif // SYMBOL

2. Une forme courante prise en charge par la plupart des compilateurs C ++, mais pas par tous, consiste à placer #pragma onceen haut du fichier d'en-tête.

L'utilisation de l'une des 2 méthodes ci-dessus pour empêcher le contenu d'un fichier d'être inclus plusieurs fois est une bonne pratique pour la programmation C ++. Cela peut améliorer les temps de compilation si le fichier est inclus plusieurs fois, cela peut également empêcher les erreurs du compilateur et les erreurs de l'éditeur de liens.

Déclarations de classe dans les fichiers d'en-tête

Pour les classes Eventet Calendar, vous définissez un destructeur pour l'objet dans la déclaration de classe, puis définissez ce destructeur dans le .cppfichier, il serait préférable de le faire dans les déclarations de classe elles-mêmes. Pour les fonctions simples ou à une seule ligne telles que, display_event()vous devez également inclure le corps de la fonction pour permettre au compilateur d'optimisation de décider si la fonction doit l'être inlinedou non.

En C ++, la section de la classe qui suit immédiatement class CLASSNAME {est privée par défaut, le mot private- clé n'est donc pas nécessaire là où vous l'avez dans votre code. Les conventions actuelles dans la programmation orientée objet sont de mettre les publicméthodes et les variables en premier, suivies par les protectedméthodes et les variables avec les privateméthodes et les variables en dernier. Cette convention est née parce que vous n'êtes peut-être pas le seul à travailler sur un projet et que quelqu'un d'autre peut avoir besoin de trouver rapidement les interfaces publiques d'une classe.

Exemple de Eventclasse refactorisée

#include <iostream>    
#include <string>

class Event {
public:
    Event(std::string eventType, std::string eventPriority, std::string eventDate,
        std::string eventTime);

    bool display_event() const {
        std::cout << "You have " << event_type << " on " << event_date << " at " << event_time << " it's " << event_priority << "\n";
        return true;
    }

    ~Event() = default;

private:
    std::string event_type;
    std::string event_priority;
    std::string event_date;
    std::string event_time;

};

Conception orientée objet

La Calendarclasse a des dépendances sur les champs privés de la Eventclasse, le problème avec ceci est qu'elle limite l'expansion du code des deux classes et rend difficile la réutilisation du code qui est une fonction principale du code orienté objet. Cela rend également le code plus difficile à maintenir. Chaque classe devrait être responsable d'une fonction / d'un travail particulier.

Vous mentionnez le tri des événements par date comme une extension possible du programme, dans ce cas, vous devez ajouter un <=opérateur pour décider dans quel ordre les événements doivent être, cet opérateur doit être dans la Eventclasse, mais il semble que vous l'implémentiez dans la classe Calendar.

Le code suivant n'appartient pas à une Calendarméthode de classe, il appartient à une Eventméthode de classe:

    day = std::stoi(event_date.substr(0, 2));
    month = std::stoi(event_date.substr(3, 2));
    year = std::stoi(event_date.substr(6, 4));
    hours = std::stoi(event_time.substr(0, 2));
    minutes = std::stoi(event_time.substr(3, 2));

Actuellement, la seule façon de créer un nouvel événement est d'essayer de l'ajouter au calendrier, il serait préférable de créer chaque événement seul, de vérifier la validité du pair puis d'appeler la méthode add_event () dans le calendrier.

Answer 2

Pour ajouter aux réponses d'Aryan Parekh et de pacmaninbw, avec lesquelles je suis d'accord:

Évitez de répéter le nom d'une classe dans ses variables membres

Par exemple, dans class Event, tous les noms de variables membres sont préfixés par event_, mais cela est redondant. Je supprimerais simplement ce préfixe.

Évitez d'utiliser à std::stringmoins que quelque chose ne soit vraiment du texte

Outre les informations de date / heure, il event_priorityy a quelque chose qui ne devrait probablement pas non plus être un std::string, mais plutôt quelque chose avec lequel le langage C ++ peut plus facilement fonctionner, comme un enum class:

class Event {
public:
    enum class Priority {
        LOW,
        NORMAL,
        URGENT,
        ...
    };

private
    std::string type; // this really look like text
    Priority priority;
    ...
};

En utilisant ce type de manière cohérente, vous devriez alors être capable d'écrire quelque chose comme:

Calendar calendar;
calendar.add_event("Exam", Event::Priority::URGENT, ...);

Une énumération est stockée sous forme d'entier, elle est donc très compacte et efficace. Il n'y a plus non plus de possibilité d'ajouter accidentellement un nom de priorité invalide ou mal orthographié, comme "ugrent". Bien sûr, vous devez maintenant ajouter des fonctions pour convertir les Prioritys en texte lisible par l'homme, donc c'est un peu plus de travail de votre part.

Passe Events directement aux fonctions membres deCalendar

Au lieu d'avoir à passer quatre paramètres à add_event(), passez simplement un seul paramètre avec type Event. Cela simplifie la mise en œuvre add_event()et la rendra pérenne. Pensez par exemple à ajouter dix autres variables membres à Event, de cette façon vous évitez d'ajouter dix paramètres supplémentaires à add_event()également! Bien sûr, assurez-vous de passer le paramètre comme constréférence:

class Calendar {
    ...
    bool add_event(const Event &event);
    ...
};

Et puis vous pouvez l'utiliser comme ceci:

Calendar calendar;
calendar.add_event({"Exam", Event::Priority::URGENT, ...});

Déplacer is_event_valid()versEvent

Vérifier si un Eventest valide relève de la responsabilité de Event. Cependant, au lieu d'avoir une fonction membre (statique) is_valid()dans class Event, envisagez de vérifier les paramètres valides dans son constructeur et faites-lui throwun std::runtime_errors'il ne peut pas le construire. De cette façon, Calendar::add_event()vous n'avez plus à faire de vérification: si vous avez réussi à le passer Event, il ne peut être valide qu'à ce stade.

L'appelant de add_event()doit gérer la possibilité pour le constructeur de class Eventlancer, mais il a déjà dû gérer le add_event()renvoi d'une erreur de toute façon, donc ce n'est pas beaucoup plus de travail.

Answer 3

En regardant votre programme, je dirais que vous avez fait un très bon travail étant donné que vous n'êtes qu'un débutant.

---

privateou public?

Jetons un coup d'œil à votre Eventclasse

class Event {
private:
    std::string event_type;
    std::string event_priority;
    std::string event_date;
    std::string event_time;

public:
    Event(std::string eventType, std::string eventPriority, std::string eventDate,
          std::string eventTime);

    bool display_event() const;

    ~Event();
};

Les 4 parties principales de votre événement, qui sont le type, la priorité, la date et l'heure, sont déclarées private.
Le problème avec ceci est que maintenant une fois que vous avez défini le Event, l'utilisateur ne peut plus le modifier. Et si le patron de l'utilisateur décide de reprogrammer une réunion, il se rend compte qu'il devrait créer un événement entièrement nouveau et supprimer le précédent. Ce ne serait pas aussi pratique que de simplement pouvoir modifier n'importe quel attribut de Event.

Un autre scénario, que vous avez mentionné dans votre question

Je pense également à ajouter une fonctionnalité permettant de trier les événements par date.

Cela signifie que vous Calendardevriez pouvoir voir les dates des événements. Mais la façon dont vous avez conçu votre classe, ce serait impossible.

Pour ces raisons, je crois qu'il Eventserait logique de définir les 4 principaux attributs du public.

---

N'encodez pas la date / l'heure comme std::string.

day = std::stoi(event_date.substr(0,2));
month = std::stoi(event_date.substr(3, 2));
year = std::stoi(event_date.substr(6, 4));
hours = std::stoi(event_time.substr(0, 2));
minutes = std::stoi(event_time.substr(3, 2));

Tous les nombres ici sont appelés nombres magiques .
Le problème avec le codage de la date / heure std::stringest que maintenant si vous voulez extraire des informations, vous devrez faire

std::stoi(event_time.substr(3, 2));

Je vous suggère de créer votre propre date/timeclasse ou même d'en utiliser une déjà définie dans la bibliothèque std :: chrono (C ++ 20).

Voici une classe de date très très simple

struct Date
{
    int day;
    int month;
    int year;
    
    date(int day, int month, int year)
        : day(day),month(month),year(year)
    {}
};

Maintenant, au lieu d'avoir à faire substrencore et encore, ce qui peut devenir bizarre. Vous pouvez facilement accéder à des parties spécifiques d'une date

Date date(25,9,2020)
// boss says push the meeting 

date.day = 30;

Notez que ce n'est qu'un exemple. Vous devez également valider la date. Un exemple d'une fonctionnalité intéressante pourrait être post_pone()qui peut prolonger une date d'un certain nombre de jours.

L'exemple s'applique pour le temps, un simple structvous permettra d'avoir plus de contrôle sur les choses et de simplifier les choses en même temps .

---

Étendre Event

Vous Eventmanquez quelques attributs.

• Lieu de l'événement
• Description de l'événement
• Durée

---

Logique if-else

bool Calendar::add_event(std::string event_type, std::string event_priority, std::string event_date,
                         std::string event_time) {

    if (is_event_valid(event_date, event_time))
    {
        Event event {std::move(event_type), std::move(event_priority), std::move(event_date), std::move(event_time)};
        calendar.push_back(event);
        return true;
    } else {
        std::cout << "Event is not valid\n";
        return false;
    }
}

Si vous inversez simplement les conditions, vous pouvez simplifier le code et supprimer une branche

bool Calendar::add_event(std::string event_type, std::string event_priority, std::string event_date,
                         std::string event_time) {

    if (!is_event_valid(event_date, event_time))
    {
        std::cout << "Event is not valid\n";
        return false;
    } 
    Event event {std::move(event_type), std::move(event_priority), std::move(event_date), std::move(event_time)};
    calendar.push_back(event);
    return true;
}

• De plus, je vous suggère d'afficher l'événement après l'avoir dit Event is not valid. Parce que si vous avez ajouté plus de 5 à 6 événements à votre calendrier, et que tout ce que vous dites était Event is not valid, vous devrez faire une recherche pour savoir quel événement

---

Étendre Calendar

Votre Calendarclasse manque certaines fonctions clés.

• Possibilité de supprimer / supprimer un événement
• Modifier un événement