I am a beginner with C++, I am not learning OOP yet, I will start when I finish this project.
I'm working on "Educational Management System Project".
So I had trouble to code the part of login and sign up, I am not comfortable with this approach, I wonder if there is a solution to simplify this code and reducing if-statement and functions in authentication.cpp file.

ems.h

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

using namespace std;

struct Person;
struct Doctor;
struct TeacherA;
struct Student;
struct Course;
struct Assignment;
struct AssignmentSolution;

// Main data
struct Person
{
int id;
string fullName;
string email;
};

struct Doctor
{
Person info;
vector <shared_ptr <Course>> courses;
};

struct TeacherA
{
Person info;
vector <shared_ptr <Course>> courses;
};

struct Student
{
Person info;
vector <shared_ptr <Course>> courses;
vector <shared_ptr <AssignmentSolution>> assignmentSolutions;
};

struct Course
{
string code;
string title;
shared_ptr <Doctor> lecturer;
shared_ptr <TeacherA> assistant;
vector <shared_ptr <Student>> registeredStudents;
vector <shared_ptr <Assignment>> assignments;
};

struct Assignment
{
string content;
int maxMark;
shared_ptr <Course> course;
vector <shared_ptr <AssignmentSolution>> assignmentSolutions;
};

struct AssignmentSolution
{
bool isMarked = false;
string solution;
int mark;
string comment = "There is no comment";
shared_ptr <Assignment> assignment;
shared_ptr <Student> student;
};
extern vector <shared_ptr <Doctor>> doctors;
extern vector <shared_ptr <TeacherA>> teachersA;
extern vector <shared_ptr <Student>> students;
extern vector <shared_ptr <Course>> courses;

int startMethod(int role);
void signUp(int role);
void signIn(int role);
int createID(int role);


ems.cpp

#include "ems.h"

vector <shared_ptr <Doctor>> doctors;
vector <shared_ptr <TeacherA>> teachersA;
vector <shared_ptr <Student>> students;
vector <shared_ptr <Course>> courses;

int main() {
cout <<"* Welcome in Educational Management System Project *\n";

}

int role = -1;
while (role) {
cout << "\nPlease enter a choice: \n"
<< "\t[1] Doctor\n"
<< "\t[2] Teacher Assistant\n"
<< "\t[3] Student\n"
<< "\t[0] Exit\n"

cin >> role;
if (role) startMethod(role);
}
}

int startMethod(int role) {
int method = -1;
while (method) {
cout << "\nPlease enter a choice: \n"
<< "\t[0] Back\n"
cin >> method;

if (method == 1)
signUp(role);
else if (method == 2)
signIn(role);
}
}


authentication.cpp

#include "ems.h"

void signUp(int role) {
Person info;
cout << "\nPlease enter the following information:\n";

cout << "Full Name: ";
getline(cin >> ws, info.fullName);

cout << "E-mail: ";
cin >> info.email;

info.id = createID(role);

}
} else {
cout << "This user already exist";
}
}

void signIn(int role) {

}
} else {
}

}

for (auto doctor: doctors) {
return true;
}
}
return false;
}

for (auto student: students) {
return true;
}
}
return false;
}

void addUserInfo(int role, Person info) {
if (role == 1) {
shared_ptr <Doctor> newDoctor (new Doctor);
newDoctor->info = info;
doctors.push_back(newDoctor);
} else if (role == 3) {
shared_ptr <Student> newStudent (new Student);
newStudent->info = info;
students.push_back(newStudent);
}
}

• Welcome to code review, you would get a better review if you posted the entire file so that we could properly review the code. Mar 23 '20 at 17:42
• @pacmaninbw I posted it, this was the first question here. Mar 23 '20 at 18:43
• @AbdelwahabHussien Why don't you use inheritance from Person for Teacher, Doctor and Student? Mar 23 '20 at 19:21
• Does the code work? If it doesn't it might be better to ask this question on stackoverflow.com rather then on code review. We only review working code. FYI, it seems that you should have a header file called authentication.h that contains the function prototypes for signUp(role); and signIn(role);. The functions createID(role) and addUserInfo(role, info); also seem to be missing. Mar 23 '20 at 19:38
• @πάνταῥεῖ I am not learning OOP yet, I will start when I finish this project. Mar 24 '20 at 2:16

## Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. Know when to use it and when not to (as when writing include headers).

## Provide complete code to reviewers

This is not so much a change to the code as a change in how you present it to other people. Without the full context of the code and an example of how to use it, it takes more effort for other people to understand your code. This affects not only code reviews, but also maintenance of the code in the future, by you or by others. One good way to address that is by the use of comments. Another good technique is to include test code showing how your code is intended to be used. For this code, I added these two functions to make it compileable:

int createID(int role) {
static int id{1000};
return ++id + role * 10000;
}

{
std::cout << "Successful login of " << username << " as role " << role << '\n';
}


## Use objects

You have a Person structure and then separate functions that operate on Person data. With only a slight syntax change, you would have a real object instead of C-style code written in C++.
So to begin with, you might define a Person class like this:

class Person
{
public:
bool isMatch(const std::string& name, const std::string& pw) const;
private:
int id;
std::string fullName;
std::string email;
};


Now we can now create a derived class to define a Doctor:

class Doctor : public Person
{
public:
Doctor(Person& p) : Person{p} {}
private:
std::vector <std::shared_ptr <Course>> courses;
};


This derivation is the Object Oriented Programming (OOP) way to express the is-a relationship. That is, a Doctor is a Person so everthing that a person has or can do, a doctor has or can do.

## Add behavior to objects for simpler code

Note too that we have defined the isMatch function for a Person. We might define it like this:

bool Person::isMatch(const std::string& name, const std::string& pw) const {
}


bool verifyDoctorData(string username, string password) {
for (auto doctor: doctors) {
return true;
}
}
return false;
}


We can write this:

bool verifyDoctorData(std::string username, std::string password) {
for (const auto& doctor: doctors) {
return true;
}
}
return false;
}


However, even better is the following suggestion.

## Use standard algorithms where practical

Do we really need to have separate verifyDoctorData and verifyStudentData? They're really nearly identical. What I'd do instead is to use std::none_of or std::any_of and the function mentioned above. So instead of this:

if (role == 1 && !verifyDoctorData(info.username, info.password)) {
}


One could write this:

if (role == 1 && std::none_of(doctors.cbegin(), doctors.cend(), std::bind(&Person::isMatch, std::placeholders::_1, username, password))) {
}


Now there is no need at all to write a verifyDoctorData function.

## Use an enum where appropriate

In the code above, instead of role == 1, wouldn't it make more sense to write role == doctor? One could do that with an enum.

There's much more, but this should be enough to get you started and to convey to you that learning C++ and learning OOP are not separate phases.

• Thanks, this was helpful to me, and of course, I'm going to start learning oop that looks powerful. Mar 25 '20 at 13:07

Well, hello. I was getting a bit bored today, so thank you for giving me a nice little exercise to work on. I edited the code, removing unnecessary lines and editing things here and there.

Since you specified that you don't yet know much about OOP I made sure not to take the easier approach, which in my mind is a vector of pointers to a virtual class named and then using it to fit in other higher derived classes in them. This is a hobby for me so I have no clue if I'm correctly communicating the concepts I'm trying to talk about, but I tried.

Part of the question was if there was a better way to reduce the number of if statements in the code that are needed by the UI to test the user's input and make selections based on them. The best way of doing that than I ever thought about is by using a map with the selections already in it as keys and having function pointers as values that can be called with a key.

There are suggestions I have for you, with a very shallow and not at all comprehensive explanation:

• Avoid using namespace std;; bad practice
• Learn OOP; It would have been more than beneficial in this case
• (Personal Opinion) Start using the c io functions printf() and scanf(); format strings make code so much neater(It's a beautiful thing)
• Try to make your program as simple as you possibly can; it makes things easier to read, edit and understand(You didn't need to have all those function declarations in the header file, and there were some unneeded std::shared_ptrs)
• (Personal Opinion) Make a main.cpp file for main(); putting it in a random file is unconventional
• Use the header guards; the errors that come with not using them are bothersome
• Usually, C++ header files are given the extension .hpp; I 99% sure it helps the compiler distinguish between C headers and C++ headers, but I'm doubting myself a little on this one.
• (For this program specifically) If you intend on using it to a degree, you should probably get write a function that can pass out the information and another to read it in; Who wants to Sign-Up to a service every time to Sign-In
• Get in the habit of writing lists of things you can improve on.

Don't worry, when I started off I got banned from stack overflow because of my cluelessness(Feel free to check out the questions I used to ask, they're fun to laugh at). I'm still banned to this day. Everyone has things they can work on. For me, it would be getting unbanned.

Anyways, the insertion code is incomplete because I got bored, so you can't actually insert any information, but I gave you a starting point for your UI and I can't do everything for you(Basically I'm too lazy to finish, but you get to learn something and I get to relax, so it's a win-win). I might get bored of relaxing and come back and finish this, but this should sufficiently help you for now.

Here's the code:

main.cpp

#include "ems.hpp"

int main(){
runEducationalManagementSystemProject();
return 0;
}


ems.hpp

#ifndef ems_hpp
#define ems_hpp

#include <vector>
#include <string>
#include <memory>

// Main data
struct Person
{
int id;
std::string fullName;
std::string email;

};

struct Assignment;
struct AssignmentSolution
{
bool isMarked = false;
std::string solution;
int mark;
std::string comment = "There is no comment";
std::shared_ptr <Assignment> assignment;
std::shared_ptr <Person> student;
};

struct Course;
struct Assignment
{
std::string content;
int maxMark;
std::shared_ptr <Course> course;
std::vector <std::shared_ptr <AssignmentSolution>> assignmentSolutions;
};

struct Course
{
std::string code;
std::string title;
std::shared_ptr <Person> lecturer;
std::shared_ptr <Person> assistant;
std::vector <std::shared_ptr <Person>> registeredStudents;
std::vector <std::shared_ptr <Assignment>> assignments;
};

// Data
extern std::vector <Person> doctors;
extern std::vector <Person> teachers;
extern std::vector <Person> students;
extern std::vector <std::shared_ptr <Course>> courses;

// Current user
extern std::string currentUser;

// Functions
void runEducationalManagementSystemProject();

#endif /* ems_hpp */


ems.cpp

#include "ems.hpp"
// Externs
std::vector <Person> doctors;
std::vector <Person> teachers;
std::vector <Person> students;

std::string currentUser;

#include <cstdio>
#include <map>
#include <string.h>

bool exists(std::string uName){
printf("Checking if \"%s\" exitsts", uName.c_str());
for (int i = 0; i < doctors.size(); i++) {
return true;
}
}

for (int i = 0; i < teachers.size(); i++) {
return true;
}
}

for (int i = 0; i < students.size(); i++) {
return true;
}
}
return false;
}

for (int i = 0; i < doctors.size(); i++) {
return true;
} else {
return false;
}
}
}

for (int i = 0; i < teachers.size(); i++) {
return true;
} else {
return false;
}
}
}

for (int i = 0; i < students.size(); i++) {
return true;
} else {
return false;
}
}
}

exit(1); // No user exists even though exits returned true, stop the program from running in needless perpetual loop
return false;
}

static std::string display;
static std::map<unsigned int, void (*)()> options;
};

while (true) {
int option = 0;
scanf("%d", &option);
MainMenu::options[option](); // Calls the function that has been connected to the key int the setup
}
}

void signUp() {
Person newUser;

char* buffer;

printf("Full Name: ");
scanf("%s", buffer);
newUser.fullName = std::string(buffer);

printf("E-mail: ");
scanf("%s", buffer);
newUser.email = std::string(buffer);

scanf("%s", buffer);
scanf("%s", buffer);
}

scanf("%s", buffer);

students.push_back(std::move(newUser));
}

void signIn() {

char* buffer;
scanf("%s", buffer);

std::string newSignIn(buffer);

int chances = 3;
tryAgain:
scanf("%s", buffer);

if (exists(newSignIn)) {
currentUser = newSignIn;
} else {
chances--;
if (chances == 0) {
printf("Sorry, you've used up all your chances\n");
return;
}
goto tryAgain;
}
} else {
printf("User does not exits, did not sign-in\n");
}
}

void quit(){
exit(0);
}

void setup(){