Text based student information system












3












$begingroup$


This is simple student information system project where you can do following things:




  1. Add Records

  2. List Records

  3. Modify Records

  4. Delete Records


To store data a .txt file is used.



I just want an honest critique of my code so I can improve.



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

using namespace std;

void flowcontrol();//directs you based on what you are trying to do;
int entrynumb();//counts the number astericks and stores them as a base for how many students they are for student number assignment;
void addrecord();//adds new students to the database;
void seerecord(string n);//lets you see the data on a student number;
void modifyrecord(string n, char c);//deletes a desired record;


int number = 0;
string strnumber = to_string(number);


struct student
{
string firstname, lastname, birthday;
int age, grade;
int number;

void writeto(student a)//writes student values to a file for retrieval and modification;
{
ofstream datastore;
datastore.open ("datastore.txt", ios_base::app);
datastore << 'n' << "Number : " << a.number;
datastore << 'n' << "Name :" << a.firstname << ' ' << a.lastname;
datastore << 'n' << "DOB : " << a.birthday;
datastore << 'n' << "Age : " << a.age;
datastore << 'n' << "Grade : " << a.grade;
datastore << 'n' << "*";
datastore << 'n' << "----------------------------------------------";

datastore.close();

}

};


int main()
{

flowcontrol();
}

void spacer(int a)//adds a certain number of newline spaces;
{
cout << string(a, 'n');
}

void flowcontrol()
{
spacer(3);
cout << "Welcome to the student database version 1.101" << 'n';
cout << "<------------------------------------------->" << 'n';
cout << " What would you like to do? " << 'n';
cout << " ADD | MODIFY | DELETE | SEE " << 'n';
cout << " a | m | d | s " << 'n';
spacer(3);

char ch; cin >> ch;

spacer(22);

switch(ch)
{
case 'a':
addrecord();
break;

case 's':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
seerecord(n);

break;
}


case 'd':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'd');

break;
}


case 'm':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'm');

break;
}

}


}

int entrynumb()//student number generator;
{
string line;//stores a line of the datastore file here for reference;
vector<string> liner;//stores the amount of entries;


ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "*")//does this if the condition is met;
{
liner.push_back("*");//pushes the asterick to the liner vector;
}
}

myfile.close();
}



return liner.size();//returns the number of astericks stored wich is directley correlated with the number of students;

}


void addrecord()//new student generator;
{

student strnumber;//initiates new student entry as a student object;

strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);
strnumber.writeto(strnumber);//calls to write the students data to datastore file;

number++;//adds the number up by one per entry to keep track of number of students;

main();//restarts the process;
}


void seerecord(string n)//takes the number provided and shows the student information;
{
string line;
ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "Number : " + n)//loops through and prints every line related associated with the number;
{ cout << " Student entry " << 'n';
cout << "<------------------------------------------->" << 'n';
cout << 'n' << line << 'n';
for(int i = 0; i < 5; i++ )
{

getline(myfile, line);
cout << line << 'n';
}

cout << "<------------------------------------------->";

}
}

myfile.close();
}

flowcontrol();
}



void modifyrecord (string n, char c)//delete//modify a desired record;
{

vector<string> datahold;//holds all the entries kept unchanged;

ifstream myfile ("datastore.txt");//opens the file;
string line;//for reference to each line;

if (myfile.is_open())
{
while (getline(myfile, line))
{

if(line == "Number : " + n)//if the number that is to be deleted is found it's associated entries wont be stored;
{

switch(c)//checks to see if your deleting or modifying a student entry;
{

case 'd'://delete;
{
for(int i = 0; i < 6; i++)
{
getline(myfile, line);
cout << "DELETING: " << line << 'n';
}

flowcontrol();

break;
}

case 'm'://edit;
{
string entries[4] = {"Name : ", "DOB : ", "Age : ", "Grade : "};

datahold.push_back(line);
for(int j = 0; j < 4; j++)//loops through and edits each entry;
{

string strcontainer;
getline(myfile, line);


cout << "Currently: " << line << 'n' << "Edit ->"; cin >> strcontainer;
datahold.push_back(entries[j] + strcontainer);
cout << "---------------------------------------->" << 'n';

}

flowcontrol();

break;
}

}

}

else
{
datahold.push_back(line);//pushes entries that won't be edited or deleted;
}

}

myfile.close();
}

ofstream myfilenew;
myfilenew.open ("datastore.txt");

for(unsigned int i = 0; i < datahold.size(); i++)//iterates through all the kept entries and rewrites them to the file;
{
myfilenew << datahold[i] << 'n';
}

}









share|improve this question











$endgroup$












  • $begingroup$
    Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:27










  • $begingroup$
    Nothing in particular. I've never had any of my code reviewed, so I just wanted some feedback on anything that I should improve on.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:31










  • $begingroup$
    It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:33










  • $begingroup$
    Okay that makes sense. I was under the impression that everything from the logic of it to the design structure would be critiqued automatically. Moving forward Ill clarify exactly what I want critiqued.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:42
















3












$begingroup$


This is simple student information system project where you can do following things:




  1. Add Records

  2. List Records

  3. Modify Records

  4. Delete Records


To store data a .txt file is used.



I just want an honest critique of my code so I can improve.



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

using namespace std;

void flowcontrol();//directs you based on what you are trying to do;
int entrynumb();//counts the number astericks and stores them as a base for how many students they are for student number assignment;
void addrecord();//adds new students to the database;
void seerecord(string n);//lets you see the data on a student number;
void modifyrecord(string n, char c);//deletes a desired record;


int number = 0;
string strnumber = to_string(number);


struct student
{
string firstname, lastname, birthday;
int age, grade;
int number;

void writeto(student a)//writes student values to a file for retrieval and modification;
{
ofstream datastore;
datastore.open ("datastore.txt", ios_base::app);
datastore << 'n' << "Number : " << a.number;
datastore << 'n' << "Name :" << a.firstname << ' ' << a.lastname;
datastore << 'n' << "DOB : " << a.birthday;
datastore << 'n' << "Age : " << a.age;
datastore << 'n' << "Grade : " << a.grade;
datastore << 'n' << "*";
datastore << 'n' << "----------------------------------------------";

datastore.close();

}

};


int main()
{

flowcontrol();
}

void spacer(int a)//adds a certain number of newline spaces;
{
cout << string(a, 'n');
}

void flowcontrol()
{
spacer(3);
cout << "Welcome to the student database version 1.101" << 'n';
cout << "<------------------------------------------->" << 'n';
cout << " What would you like to do? " << 'n';
cout << " ADD | MODIFY | DELETE | SEE " << 'n';
cout << " a | m | d | s " << 'n';
spacer(3);

char ch; cin >> ch;

spacer(22);

switch(ch)
{
case 'a':
addrecord();
break;

case 's':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
seerecord(n);

break;
}


case 'd':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'd');

break;
}


case 'm':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'm');

break;
}

}


}

int entrynumb()//student number generator;
{
string line;//stores a line of the datastore file here for reference;
vector<string> liner;//stores the amount of entries;


ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "*")//does this if the condition is met;
{
liner.push_back("*");//pushes the asterick to the liner vector;
}
}

myfile.close();
}



return liner.size();//returns the number of astericks stored wich is directley correlated with the number of students;

}


void addrecord()//new student generator;
{

student strnumber;//initiates new student entry as a student object;

strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);
strnumber.writeto(strnumber);//calls to write the students data to datastore file;

number++;//adds the number up by one per entry to keep track of number of students;

main();//restarts the process;
}


void seerecord(string n)//takes the number provided and shows the student information;
{
string line;
ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "Number : " + n)//loops through and prints every line related associated with the number;
{ cout << " Student entry " << 'n';
cout << "<------------------------------------------->" << 'n';
cout << 'n' << line << 'n';
for(int i = 0; i < 5; i++ )
{

getline(myfile, line);
cout << line << 'n';
}

cout << "<------------------------------------------->";

}
}

myfile.close();
}

flowcontrol();
}



void modifyrecord (string n, char c)//delete//modify a desired record;
{

vector<string> datahold;//holds all the entries kept unchanged;

ifstream myfile ("datastore.txt");//opens the file;
string line;//for reference to each line;

if (myfile.is_open())
{
while (getline(myfile, line))
{

if(line == "Number : " + n)//if the number that is to be deleted is found it's associated entries wont be stored;
{

switch(c)//checks to see if your deleting or modifying a student entry;
{

case 'd'://delete;
{
for(int i = 0; i < 6; i++)
{
getline(myfile, line);
cout << "DELETING: " << line << 'n';
}

flowcontrol();

break;
}

case 'm'://edit;
{
string entries[4] = {"Name : ", "DOB : ", "Age : ", "Grade : "};

datahold.push_back(line);
for(int j = 0; j < 4; j++)//loops through and edits each entry;
{

string strcontainer;
getline(myfile, line);


cout << "Currently: " << line << 'n' << "Edit ->"; cin >> strcontainer;
datahold.push_back(entries[j] + strcontainer);
cout << "---------------------------------------->" << 'n';

}

flowcontrol();

break;
}

}

}

else
{
datahold.push_back(line);//pushes entries that won't be edited or deleted;
}

}

myfile.close();
}

ofstream myfilenew;
myfilenew.open ("datastore.txt");

for(unsigned int i = 0; i < datahold.size(); i++)//iterates through all the kept entries and rewrites them to the file;
{
myfilenew << datahold[i] << 'n';
}

}









share|improve this question











$endgroup$












  • $begingroup$
    Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:27










  • $begingroup$
    Nothing in particular. I've never had any of my code reviewed, so I just wanted some feedback on anything that I should improve on.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:31










  • $begingroup$
    It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:33










  • $begingroup$
    Okay that makes sense. I was under the impression that everything from the logic of it to the design structure would be critiqued automatically. Moving forward Ill clarify exactly what I want critiqued.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:42














3












3








3





$begingroup$


This is simple student information system project where you can do following things:




  1. Add Records

  2. List Records

  3. Modify Records

  4. Delete Records


To store data a .txt file is used.



I just want an honest critique of my code so I can improve.



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

using namespace std;

void flowcontrol();//directs you based on what you are trying to do;
int entrynumb();//counts the number astericks and stores them as a base for how many students they are for student number assignment;
void addrecord();//adds new students to the database;
void seerecord(string n);//lets you see the data on a student number;
void modifyrecord(string n, char c);//deletes a desired record;


int number = 0;
string strnumber = to_string(number);


struct student
{
string firstname, lastname, birthday;
int age, grade;
int number;

void writeto(student a)//writes student values to a file for retrieval and modification;
{
ofstream datastore;
datastore.open ("datastore.txt", ios_base::app);
datastore << 'n' << "Number : " << a.number;
datastore << 'n' << "Name :" << a.firstname << ' ' << a.lastname;
datastore << 'n' << "DOB : " << a.birthday;
datastore << 'n' << "Age : " << a.age;
datastore << 'n' << "Grade : " << a.grade;
datastore << 'n' << "*";
datastore << 'n' << "----------------------------------------------";

datastore.close();

}

};


int main()
{

flowcontrol();
}

void spacer(int a)//adds a certain number of newline spaces;
{
cout << string(a, 'n');
}

void flowcontrol()
{
spacer(3);
cout << "Welcome to the student database version 1.101" << 'n';
cout << "<------------------------------------------->" << 'n';
cout << " What would you like to do? " << 'n';
cout << " ADD | MODIFY | DELETE | SEE " << 'n';
cout << " a | m | d | s " << 'n';
spacer(3);

char ch; cin >> ch;

spacer(22);

switch(ch)
{
case 'a':
addrecord();
break;

case 's':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
seerecord(n);

break;
}


case 'd':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'd');

break;
}


case 'm':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'm');

break;
}

}


}

int entrynumb()//student number generator;
{
string line;//stores a line of the datastore file here for reference;
vector<string> liner;//stores the amount of entries;


ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "*")//does this if the condition is met;
{
liner.push_back("*");//pushes the asterick to the liner vector;
}
}

myfile.close();
}



return liner.size();//returns the number of astericks stored wich is directley correlated with the number of students;

}


void addrecord()//new student generator;
{

student strnumber;//initiates new student entry as a student object;

strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);
strnumber.writeto(strnumber);//calls to write the students data to datastore file;

number++;//adds the number up by one per entry to keep track of number of students;

main();//restarts the process;
}


void seerecord(string n)//takes the number provided and shows the student information;
{
string line;
ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "Number : " + n)//loops through and prints every line related associated with the number;
{ cout << " Student entry " << 'n';
cout << "<------------------------------------------->" << 'n';
cout << 'n' << line << 'n';
for(int i = 0; i < 5; i++ )
{

getline(myfile, line);
cout << line << 'n';
}

cout << "<------------------------------------------->";

}
}

myfile.close();
}

flowcontrol();
}



void modifyrecord (string n, char c)//delete//modify a desired record;
{

vector<string> datahold;//holds all the entries kept unchanged;

ifstream myfile ("datastore.txt");//opens the file;
string line;//for reference to each line;

if (myfile.is_open())
{
while (getline(myfile, line))
{

if(line == "Number : " + n)//if the number that is to be deleted is found it's associated entries wont be stored;
{

switch(c)//checks to see if your deleting or modifying a student entry;
{

case 'd'://delete;
{
for(int i = 0; i < 6; i++)
{
getline(myfile, line);
cout << "DELETING: " << line << 'n';
}

flowcontrol();

break;
}

case 'm'://edit;
{
string entries[4] = {"Name : ", "DOB : ", "Age : ", "Grade : "};

datahold.push_back(line);
for(int j = 0; j < 4; j++)//loops through and edits each entry;
{

string strcontainer;
getline(myfile, line);


cout << "Currently: " << line << 'n' << "Edit ->"; cin >> strcontainer;
datahold.push_back(entries[j] + strcontainer);
cout << "---------------------------------------->" << 'n';

}

flowcontrol();

break;
}

}

}

else
{
datahold.push_back(line);//pushes entries that won't be edited or deleted;
}

}

myfile.close();
}

ofstream myfilenew;
myfilenew.open ("datastore.txt");

for(unsigned int i = 0; i < datahold.size(); i++)//iterates through all the kept entries and rewrites them to the file;
{
myfilenew << datahold[i] << 'n';
}

}









share|improve this question











$endgroup$




This is simple student information system project where you can do following things:




  1. Add Records

  2. List Records

  3. Modify Records

  4. Delete Records


To store data a .txt file is used.



I just want an honest critique of my code so I can improve.



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

using namespace std;

void flowcontrol();//directs you based on what you are trying to do;
int entrynumb();//counts the number astericks and stores them as a base for how many students they are for student number assignment;
void addrecord();//adds new students to the database;
void seerecord(string n);//lets you see the data on a student number;
void modifyrecord(string n, char c);//deletes a desired record;


int number = 0;
string strnumber = to_string(number);


struct student
{
string firstname, lastname, birthday;
int age, grade;
int number;

void writeto(student a)//writes student values to a file for retrieval and modification;
{
ofstream datastore;
datastore.open ("datastore.txt", ios_base::app);
datastore << 'n' << "Number : " << a.number;
datastore << 'n' << "Name :" << a.firstname << ' ' << a.lastname;
datastore << 'n' << "DOB : " << a.birthday;
datastore << 'n' << "Age : " << a.age;
datastore << 'n' << "Grade : " << a.grade;
datastore << 'n' << "*";
datastore << 'n' << "----------------------------------------------";

datastore.close();

}

};


int main()
{

flowcontrol();
}

void spacer(int a)//adds a certain number of newline spaces;
{
cout << string(a, 'n');
}

void flowcontrol()
{
spacer(3);
cout << "Welcome to the student database version 1.101" << 'n';
cout << "<------------------------------------------->" << 'n';
cout << " What would you like to do? " << 'n';
cout << " ADD | MODIFY | DELETE | SEE " << 'n';
cout << " a | m | d | s " << 'n';
spacer(3);

char ch; cin >> ch;

spacer(22);

switch(ch)
{
case 'a':
addrecord();
break;

case 's':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
seerecord(n);

break;
}


case 'd':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'd');

break;
}


case 'm':
{
spacer(2);
cout << "Student ID number : " << 'n';
string n; cin >> n;
spacer(5);
modifyrecord(n, 'm');

break;
}

}


}

int entrynumb()//student number generator;
{
string line;//stores a line of the datastore file here for reference;
vector<string> liner;//stores the amount of entries;


ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "*")//does this if the condition is met;
{
liner.push_back("*");//pushes the asterick to the liner vector;
}
}

myfile.close();
}



return liner.size();//returns the number of astericks stored wich is directley correlated with the number of students;

}


void addrecord()//new student generator;
{

student strnumber;//initiates new student entry as a student object;

strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);
strnumber.writeto(strnumber);//calls to write the students data to datastore file;

number++;//adds the number up by one per entry to keep track of number of students;

main();//restarts the process;
}


void seerecord(string n)//takes the number provided and shows the student information;
{
string line;
ifstream myfile ("datastore.txt");//opens the datastore file;
if (myfile.is_open())
{
while (getline(myfile,line))//grabs the lines in datastore and does something;
{

if(line == "Number : " + n)//loops through and prints every line related associated with the number;
{ cout << " Student entry " << 'n';
cout << "<------------------------------------------->" << 'n';
cout << 'n' << line << 'n';
for(int i = 0; i < 5; i++ )
{

getline(myfile, line);
cout << line << 'n';
}

cout << "<------------------------------------------->";

}
}

myfile.close();
}

flowcontrol();
}



void modifyrecord (string n, char c)//delete//modify a desired record;
{

vector<string> datahold;//holds all the entries kept unchanged;

ifstream myfile ("datastore.txt");//opens the file;
string line;//for reference to each line;

if (myfile.is_open())
{
while (getline(myfile, line))
{

if(line == "Number : " + n)//if the number that is to be deleted is found it's associated entries wont be stored;
{

switch(c)//checks to see if your deleting or modifying a student entry;
{

case 'd'://delete;
{
for(int i = 0; i < 6; i++)
{
getline(myfile, line);
cout << "DELETING: " << line << 'n';
}

flowcontrol();

break;
}

case 'm'://edit;
{
string entries[4] = {"Name : ", "DOB : ", "Age : ", "Grade : "};

datahold.push_back(line);
for(int j = 0; j < 4; j++)//loops through and edits each entry;
{

string strcontainer;
getline(myfile, line);


cout << "Currently: " << line << 'n' << "Edit ->"; cin >> strcontainer;
datahold.push_back(entries[j] + strcontainer);
cout << "---------------------------------------->" << 'n';

}

flowcontrol();

break;
}

}

}

else
{
datahold.push_back(line);//pushes entries that won't be edited or deleted;
}

}

myfile.close();
}

ofstream myfilenew;
myfilenew.open ("datastore.txt");

for(unsigned int i = 0; i < datahold.size(); i++)//iterates through all the kept entries and rewrites them to the file;
{
myfilenew << datahold[i] << 'n';
}

}






c++ c++11






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Dec 29 '18 at 3:19









Reinderien

4,180822




4,180822










asked Dec 29 '18 at 1:44









VildjhartaVildjharta

184




184












  • $begingroup$
    Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:27










  • $begingroup$
    Nothing in particular. I've never had any of my code reviewed, so I just wanted some feedback on anything that I should improve on.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:31










  • $begingroup$
    It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:33










  • $begingroup$
    Okay that makes sense. I was under the impression that everything from the logic of it to the design structure would be critiqued automatically. Moving forward Ill clarify exactly what I want critiqued.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:42


















  • $begingroup$
    Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:27










  • $begingroup$
    Nothing in particular. I've never had any of my code reviewed, so I just wanted some feedback on anything that I should improve on.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:31










  • $begingroup$
    It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    $endgroup$
    – mickmackusa
    Dec 29 '18 at 2:33










  • $begingroup$
    Okay that makes sense. I was under the impression that everything from the logic of it to the design structure would be critiqued automatically. Moving forward Ill clarify exactly what I want critiqued.
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 2:42
















$begingroup$
Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
$endgroup$
– mickmackusa
Dec 29 '18 at 2:27




$begingroup$
Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
$endgroup$
– mickmackusa
Dec 29 '18 at 2:27












$begingroup$
Nothing in particular. I've never had any of my code reviewed, so I just wanted some feedback on anything that I should improve on.
$endgroup$
– Vildjharta
Dec 29 '18 at 2:31




$begingroup$
Nothing in particular. I've never had any of my code reviewed, so I just wanted some feedback on anything that I should improve on.
$endgroup$
– Vildjharta
Dec 29 '18 at 2:31












$begingroup$
It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
$endgroup$
– mickmackusa
Dec 29 '18 at 2:33




$begingroup$
It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
$endgroup$
– mickmackusa
Dec 29 '18 at 2:33












$begingroup$
Okay that makes sense. I was under the impression that everything from the logic of it to the design structure would be critiqued automatically. Moving forward Ill clarify exactly what I want critiqued.
$endgroup$
– Vildjharta
Dec 29 '18 at 2:42




$begingroup$
Okay that makes sense. I was under the impression that everything from the logic of it to the design structure would be critiqued automatically. Moving forward Ill clarify exactly what I want critiqued.
$endgroup$
– Vildjharta
Dec 29 '18 at 2:42










2 Answers
2






active

oldest

votes


















5












$begingroup$

Here are some observations that may help you improve your code.



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.



Return something useful from the subroutines



Almost every single one of the routines is declared as returning void. Something is wrong there. For example, if addrecord fails for some reason such as invalid input data, it would be nice if the function returned a value indicating this fact.



Eliminate global variables



My rewrite of this code uses no global variables, so clearly they are neither faster nor necessary. Eliminating them allows your code to be more readable and maintainable, both of which are important characteristics of well-written code. Global variables introduce messy linkages that are difficult to spot and error prone.



Use better naming



The name flowcontrol is not really helpful, since it does much more than simply reading -- all of the processing, including output, is done there. Names like line are fine in context, but myfile is not such a good name. Variable and function names should tell the reader why they exist, ideally without further comments necessary. So instead of spacer() I'd probably name it print_empty_lines() for example.



Use string concatenation



The menu includes these lines:



std::cout << "Welcome to the student database version 1.101" << 'n';
std::cout << "<------------------------------------------->" << 'n';
std::cout << " What would you like to do? " << 'n';


Each of those is a separate call to operator<< but they don't need to be. Another way to write that would be like this:



std::cout 
<< "Welcome to the student database version 1.101n"
<< "<------------------------------------------->n"
<< " What would you like to do? n"
// etc.


This reduces the entire menu to a single call to operator<< because consecutive strings in C++ (and in C, for that matter) are automatically concatenated into a single string by the compiler.



Don't duplicate important constants



The filename is hardcoded right now (see next suggestion), but worse than that, it's done in five completely indpendent places. Better would be to create a constant:



static const char *FILENAME = "namelist.txt";


Consider the user



Instead of having a hardcoded filename, it might be nice to allow the user to control the name and location of the file. For this, it would make sense to use a command line argument and then pass the filename to the functions as needed.



Make better use of objects



The only member function of student is writeto which is not a good function name and may as well be a static function by the way it's written. Instead, I'd suggest something like this:



bool appendTo(const std::string &filename) const {
std::ofstream datastore{"datastore.txt", std::ios_base::app};
if (datastore) {
datastore <<
"Number : " << number
"nName :" << firstname << ' ' << lastname
"nDOB : " << birthday
"nAge : " << age
"nGrade : "<< grade
"n*"
"n----------------------------------------------n";
}
bool result{datastore};
return result;
}


Note that now it takes the filename as a parameter and return true if the write was successful. It is also const because it doesn't alter the underlying student data structure.



Understand the uniqueness of main



The end of flowcontrol() is this:



    main();//restarts the process;
}


However, that's not legal C++. You can't take the address of main nor call it. It may seem to work with your compiler but it's explicitly not guaranteed by the standard and is undefined behavior meaning that the program might do anything and that it's not predictable. If you need to repeat something, use a loop.






share|improve this answer









$endgroup$













  • $begingroup$
    Is there a rule of thumb for when I should use void ? Or is it just better design to return values for error handling
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:45








  • 1




    $begingroup$
    Generally, if it doesn’t return something obvious like entrynumb and can’t fail (or throws an exception on failure) then void is ok. However if all of your functions return void it’s probably a sign that a little more thought about the design would be beneficial.
    $endgroup$
    – Edward
    Dec 29 '18 at 19:56










  • $begingroup$
    Thank you for your feedback I will keep that in mind during my edit/moving forward!
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 20:00



















4












$begingroup$

These comments:



//directs you based on what you are trying to do;


don't need to end in semicolons. You're speaking English rather than C++ inside of them.




astericks




is spelled asterisks.



This code:



int number = 0;
string strnumber = to_string(number);


seems out-of-place. If it's just test code, delete it.



struct student


First, you'll probably want to capitalize Student. Your writeto method should be declared as



void writeto(ostream &datastore) const {


In particular, it shouldn't accept a, rather using this to identify the student; and it shouldn't open the file itself. It should be able to write to any stream (including cout).



For code like this:



char ch; cin >> ch;


Try to avoid adding multiple statements on one line, for legibility's sake.



A large section of your addrecord method should be moved to a method on Student. Specifically, this:



strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);


can be made into a method perhaps called initFromPrompt that initializes the current Student instance. Also, your "container" input pattern doesn't need to exist. For instance, for grade, simply



cin >> strnumber.grade;


This:



main();//restarts the process;


is misdesigned. You definitely don't want to be calling main yourself, for many reasons - including recursion. If you want to "restart the process", you should make some kind of top-level loop.



This:



ifstream myfile ("datastore.txt");//opens the file;
if (myfile.is_open())


is somewhat of an anti-pattern. This kind of code is going to lead to silent errors. Instead, read about how to enable std::ios::failbit so that you get exceptions from bad stream calls.






share|improve this answer









$endgroup$













  • $begingroup$
    Can you give me an example of how I would call the write to method after your edit? I'm having a hard time understanding how it would work
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:51






  • 1




    $begingroup$
    strnumber.writeto(datastore), where datastore is your opened file and strnumber is your student instance
    $endgroup$
    – Reinderien
    Dec 29 '18 at 23:40











Your Answer





StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");

StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});


}
});














draft saved

draft discarded


















StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f210535%2ftext-based-student-information-system%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown

























2 Answers
2






active

oldest

votes








2 Answers
2






active

oldest

votes









active

oldest

votes






active

oldest

votes









5












$begingroup$

Here are some observations that may help you improve your code.



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.



Return something useful from the subroutines



Almost every single one of the routines is declared as returning void. Something is wrong there. For example, if addrecord fails for some reason such as invalid input data, it would be nice if the function returned a value indicating this fact.



Eliminate global variables



My rewrite of this code uses no global variables, so clearly they are neither faster nor necessary. Eliminating them allows your code to be more readable and maintainable, both of which are important characteristics of well-written code. Global variables introduce messy linkages that are difficult to spot and error prone.



Use better naming



The name flowcontrol is not really helpful, since it does much more than simply reading -- all of the processing, including output, is done there. Names like line are fine in context, but myfile is not such a good name. Variable and function names should tell the reader why they exist, ideally without further comments necessary. So instead of spacer() I'd probably name it print_empty_lines() for example.



Use string concatenation



The menu includes these lines:



std::cout << "Welcome to the student database version 1.101" << 'n';
std::cout << "<------------------------------------------->" << 'n';
std::cout << " What would you like to do? " << 'n';


Each of those is a separate call to operator<< but they don't need to be. Another way to write that would be like this:



std::cout 
<< "Welcome to the student database version 1.101n"
<< "<------------------------------------------->n"
<< " What would you like to do? n"
// etc.


This reduces the entire menu to a single call to operator<< because consecutive strings in C++ (and in C, for that matter) are automatically concatenated into a single string by the compiler.



Don't duplicate important constants



The filename is hardcoded right now (see next suggestion), but worse than that, it's done in five completely indpendent places. Better would be to create a constant:



static const char *FILENAME = "namelist.txt";


Consider the user



Instead of having a hardcoded filename, it might be nice to allow the user to control the name and location of the file. For this, it would make sense to use a command line argument and then pass the filename to the functions as needed.



Make better use of objects



The only member function of student is writeto which is not a good function name and may as well be a static function by the way it's written. Instead, I'd suggest something like this:



bool appendTo(const std::string &filename) const {
std::ofstream datastore{"datastore.txt", std::ios_base::app};
if (datastore) {
datastore <<
"Number : " << number
"nName :" << firstname << ' ' << lastname
"nDOB : " << birthday
"nAge : " << age
"nGrade : "<< grade
"n*"
"n----------------------------------------------n";
}
bool result{datastore};
return result;
}


Note that now it takes the filename as a parameter and return true if the write was successful. It is also const because it doesn't alter the underlying student data structure.



Understand the uniqueness of main



The end of flowcontrol() is this:



    main();//restarts the process;
}


However, that's not legal C++. You can't take the address of main nor call it. It may seem to work with your compiler but it's explicitly not guaranteed by the standard and is undefined behavior meaning that the program might do anything and that it's not predictable. If you need to repeat something, use a loop.






share|improve this answer









$endgroup$













  • $begingroup$
    Is there a rule of thumb for when I should use void ? Or is it just better design to return values for error handling
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:45








  • 1




    $begingroup$
    Generally, if it doesn’t return something obvious like entrynumb and can’t fail (or throws an exception on failure) then void is ok. However if all of your functions return void it’s probably a sign that a little more thought about the design would be beneficial.
    $endgroup$
    – Edward
    Dec 29 '18 at 19:56










  • $begingroup$
    Thank you for your feedback I will keep that in mind during my edit/moving forward!
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 20:00
















5












$begingroup$

Here are some observations that may help you improve your code.



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.



Return something useful from the subroutines



Almost every single one of the routines is declared as returning void. Something is wrong there. For example, if addrecord fails for some reason such as invalid input data, it would be nice if the function returned a value indicating this fact.



Eliminate global variables



My rewrite of this code uses no global variables, so clearly they are neither faster nor necessary. Eliminating them allows your code to be more readable and maintainable, both of which are important characteristics of well-written code. Global variables introduce messy linkages that are difficult to spot and error prone.



Use better naming



The name flowcontrol is not really helpful, since it does much more than simply reading -- all of the processing, including output, is done there. Names like line are fine in context, but myfile is not such a good name. Variable and function names should tell the reader why they exist, ideally without further comments necessary. So instead of spacer() I'd probably name it print_empty_lines() for example.



Use string concatenation



The menu includes these lines:



std::cout << "Welcome to the student database version 1.101" << 'n';
std::cout << "<------------------------------------------->" << 'n';
std::cout << " What would you like to do? " << 'n';


Each of those is a separate call to operator<< but they don't need to be. Another way to write that would be like this:



std::cout 
<< "Welcome to the student database version 1.101n"
<< "<------------------------------------------->n"
<< " What would you like to do? n"
// etc.


This reduces the entire menu to a single call to operator<< because consecutive strings in C++ (and in C, for that matter) are automatically concatenated into a single string by the compiler.



Don't duplicate important constants



The filename is hardcoded right now (see next suggestion), but worse than that, it's done in five completely indpendent places. Better would be to create a constant:



static const char *FILENAME = "namelist.txt";


Consider the user



Instead of having a hardcoded filename, it might be nice to allow the user to control the name and location of the file. For this, it would make sense to use a command line argument and then pass the filename to the functions as needed.



Make better use of objects



The only member function of student is writeto which is not a good function name and may as well be a static function by the way it's written. Instead, I'd suggest something like this:



bool appendTo(const std::string &filename) const {
std::ofstream datastore{"datastore.txt", std::ios_base::app};
if (datastore) {
datastore <<
"Number : " << number
"nName :" << firstname << ' ' << lastname
"nDOB : " << birthday
"nAge : " << age
"nGrade : "<< grade
"n*"
"n----------------------------------------------n";
}
bool result{datastore};
return result;
}


Note that now it takes the filename as a parameter and return true if the write was successful. It is also const because it doesn't alter the underlying student data structure.



Understand the uniqueness of main



The end of flowcontrol() is this:



    main();//restarts the process;
}


However, that's not legal C++. You can't take the address of main nor call it. It may seem to work with your compiler but it's explicitly not guaranteed by the standard and is undefined behavior meaning that the program might do anything and that it's not predictable. If you need to repeat something, use a loop.






share|improve this answer









$endgroup$













  • $begingroup$
    Is there a rule of thumb for when I should use void ? Or is it just better design to return values for error handling
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:45








  • 1




    $begingroup$
    Generally, if it doesn’t return something obvious like entrynumb and can’t fail (or throws an exception on failure) then void is ok. However if all of your functions return void it’s probably a sign that a little more thought about the design would be beneficial.
    $endgroup$
    – Edward
    Dec 29 '18 at 19:56










  • $begingroup$
    Thank you for your feedback I will keep that in mind during my edit/moving forward!
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 20:00














5












5








5





$begingroup$

Here are some observations that may help you improve your code.



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.



Return something useful from the subroutines



Almost every single one of the routines is declared as returning void. Something is wrong there. For example, if addrecord fails for some reason such as invalid input data, it would be nice if the function returned a value indicating this fact.



Eliminate global variables



My rewrite of this code uses no global variables, so clearly they are neither faster nor necessary. Eliminating them allows your code to be more readable and maintainable, both of which are important characteristics of well-written code. Global variables introduce messy linkages that are difficult to spot and error prone.



Use better naming



The name flowcontrol is not really helpful, since it does much more than simply reading -- all of the processing, including output, is done there. Names like line are fine in context, but myfile is not such a good name. Variable and function names should tell the reader why they exist, ideally without further comments necessary. So instead of spacer() I'd probably name it print_empty_lines() for example.



Use string concatenation



The menu includes these lines:



std::cout << "Welcome to the student database version 1.101" << 'n';
std::cout << "<------------------------------------------->" << 'n';
std::cout << " What would you like to do? " << 'n';


Each of those is a separate call to operator<< but they don't need to be. Another way to write that would be like this:



std::cout 
<< "Welcome to the student database version 1.101n"
<< "<------------------------------------------->n"
<< " What would you like to do? n"
// etc.


This reduces the entire menu to a single call to operator<< because consecutive strings in C++ (and in C, for that matter) are automatically concatenated into a single string by the compiler.



Don't duplicate important constants



The filename is hardcoded right now (see next suggestion), but worse than that, it's done in five completely indpendent places. Better would be to create a constant:



static const char *FILENAME = "namelist.txt";


Consider the user



Instead of having a hardcoded filename, it might be nice to allow the user to control the name and location of the file. For this, it would make sense to use a command line argument and then pass the filename to the functions as needed.



Make better use of objects



The only member function of student is writeto which is not a good function name and may as well be a static function by the way it's written. Instead, I'd suggest something like this:



bool appendTo(const std::string &filename) const {
std::ofstream datastore{"datastore.txt", std::ios_base::app};
if (datastore) {
datastore <<
"Number : " << number
"nName :" << firstname << ' ' << lastname
"nDOB : " << birthday
"nAge : " << age
"nGrade : "<< grade
"n*"
"n----------------------------------------------n";
}
bool result{datastore};
return result;
}


Note that now it takes the filename as a parameter and return true if the write was successful. It is also const because it doesn't alter the underlying student data structure.



Understand the uniqueness of main



The end of flowcontrol() is this:



    main();//restarts the process;
}


However, that's not legal C++. You can't take the address of main nor call it. It may seem to work with your compiler but it's explicitly not guaranteed by the standard and is undefined behavior meaning that the program might do anything and that it's not predictable. If you need to repeat something, use a loop.






share|improve this answer









$endgroup$



Here are some observations that may help you improve your code.



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.



Return something useful from the subroutines



Almost every single one of the routines is declared as returning void. Something is wrong there. For example, if addrecord fails for some reason such as invalid input data, it would be nice if the function returned a value indicating this fact.



Eliminate global variables



My rewrite of this code uses no global variables, so clearly they are neither faster nor necessary. Eliminating them allows your code to be more readable and maintainable, both of which are important characteristics of well-written code. Global variables introduce messy linkages that are difficult to spot and error prone.



Use better naming



The name flowcontrol is not really helpful, since it does much more than simply reading -- all of the processing, including output, is done there. Names like line are fine in context, but myfile is not such a good name. Variable and function names should tell the reader why they exist, ideally without further comments necessary. So instead of spacer() I'd probably name it print_empty_lines() for example.



Use string concatenation



The menu includes these lines:



std::cout << "Welcome to the student database version 1.101" << 'n';
std::cout << "<------------------------------------------->" << 'n';
std::cout << " What would you like to do? " << 'n';


Each of those is a separate call to operator<< but they don't need to be. Another way to write that would be like this:



std::cout 
<< "Welcome to the student database version 1.101n"
<< "<------------------------------------------->n"
<< " What would you like to do? n"
// etc.


This reduces the entire menu to a single call to operator<< because consecutive strings in C++ (and in C, for that matter) are automatically concatenated into a single string by the compiler.



Don't duplicate important constants



The filename is hardcoded right now (see next suggestion), but worse than that, it's done in five completely indpendent places. Better would be to create a constant:



static const char *FILENAME = "namelist.txt";


Consider the user



Instead of having a hardcoded filename, it might be nice to allow the user to control the name and location of the file. For this, it would make sense to use a command line argument and then pass the filename to the functions as needed.



Make better use of objects



The only member function of student is writeto which is not a good function name and may as well be a static function by the way it's written. Instead, I'd suggest something like this:



bool appendTo(const std::string &filename) const {
std::ofstream datastore{"datastore.txt", std::ios_base::app};
if (datastore) {
datastore <<
"Number : " << number
"nName :" << firstname << ' ' << lastname
"nDOB : " << birthday
"nAge : " << age
"nGrade : "<< grade
"n*"
"n----------------------------------------------n";
}
bool result{datastore};
return result;
}


Note that now it takes the filename as a parameter and return true if the write was successful. It is also const because it doesn't alter the underlying student data structure.



Understand the uniqueness of main



The end of flowcontrol() is this:



    main();//restarts the process;
}


However, that's not legal C++. You can't take the address of main nor call it. It may seem to work with your compiler but it's explicitly not guaranteed by the standard and is undefined behavior meaning that the program might do anything and that it's not predictable. If you need to repeat something, use a loop.







share|improve this answer












share|improve this answer



share|improve this answer










answered Dec 29 '18 at 3:42









EdwardEdward

46.8k378212




46.8k378212












  • $begingroup$
    Is there a rule of thumb for when I should use void ? Or is it just better design to return values for error handling
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:45








  • 1




    $begingroup$
    Generally, if it doesn’t return something obvious like entrynumb and can’t fail (or throws an exception on failure) then void is ok. However if all of your functions return void it’s probably a sign that a little more thought about the design would be beneficial.
    $endgroup$
    – Edward
    Dec 29 '18 at 19:56










  • $begingroup$
    Thank you for your feedback I will keep that in mind during my edit/moving forward!
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 20:00


















  • $begingroup$
    Is there a rule of thumb for when I should use void ? Or is it just better design to return values for error handling
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:45








  • 1




    $begingroup$
    Generally, if it doesn’t return something obvious like entrynumb and can’t fail (or throws an exception on failure) then void is ok. However if all of your functions return void it’s probably a sign that a little more thought about the design would be beneficial.
    $endgroup$
    – Edward
    Dec 29 '18 at 19:56










  • $begingroup$
    Thank you for your feedback I will keep that in mind during my edit/moving forward!
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 20:00
















$begingroup$
Is there a rule of thumb for when I should use void ? Or is it just better design to return values for error handling
$endgroup$
– Vildjharta
Dec 29 '18 at 19:45






$begingroup$
Is there a rule of thumb for when I should use void ? Or is it just better design to return values for error handling
$endgroup$
– Vildjharta
Dec 29 '18 at 19:45






1




1




$begingroup$
Generally, if it doesn’t return something obvious like entrynumb and can’t fail (or throws an exception on failure) then void is ok. However if all of your functions return void it’s probably a sign that a little more thought about the design would be beneficial.
$endgroup$
– Edward
Dec 29 '18 at 19:56




$begingroup$
Generally, if it doesn’t return something obvious like entrynumb and can’t fail (or throws an exception on failure) then void is ok. However if all of your functions return void it’s probably a sign that a little more thought about the design would be beneficial.
$endgroup$
– Edward
Dec 29 '18 at 19:56












$begingroup$
Thank you for your feedback I will keep that in mind during my edit/moving forward!
$endgroup$
– Vildjharta
Dec 29 '18 at 20:00




$begingroup$
Thank you for your feedback I will keep that in mind during my edit/moving forward!
$endgroup$
– Vildjharta
Dec 29 '18 at 20:00













4












$begingroup$

These comments:



//directs you based on what you are trying to do;


don't need to end in semicolons. You're speaking English rather than C++ inside of them.




astericks




is spelled asterisks.



This code:



int number = 0;
string strnumber = to_string(number);


seems out-of-place. If it's just test code, delete it.



struct student


First, you'll probably want to capitalize Student. Your writeto method should be declared as



void writeto(ostream &datastore) const {


In particular, it shouldn't accept a, rather using this to identify the student; and it shouldn't open the file itself. It should be able to write to any stream (including cout).



For code like this:



char ch; cin >> ch;


Try to avoid adding multiple statements on one line, for legibility's sake.



A large section of your addrecord method should be moved to a method on Student. Specifically, this:



strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);


can be made into a method perhaps called initFromPrompt that initializes the current Student instance. Also, your "container" input pattern doesn't need to exist. For instance, for grade, simply



cin >> strnumber.grade;


This:



main();//restarts the process;


is misdesigned. You definitely don't want to be calling main yourself, for many reasons - including recursion. If you want to "restart the process", you should make some kind of top-level loop.



This:



ifstream myfile ("datastore.txt");//opens the file;
if (myfile.is_open())


is somewhat of an anti-pattern. This kind of code is going to lead to silent errors. Instead, read about how to enable std::ios::failbit so that you get exceptions from bad stream calls.






share|improve this answer









$endgroup$













  • $begingroup$
    Can you give me an example of how I would call the write to method after your edit? I'm having a hard time understanding how it would work
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:51






  • 1




    $begingroup$
    strnumber.writeto(datastore), where datastore is your opened file and strnumber is your student instance
    $endgroup$
    – Reinderien
    Dec 29 '18 at 23:40
















4












$begingroup$

These comments:



//directs you based on what you are trying to do;


don't need to end in semicolons. You're speaking English rather than C++ inside of them.




astericks




is spelled asterisks.



This code:



int number = 0;
string strnumber = to_string(number);


seems out-of-place. If it's just test code, delete it.



struct student


First, you'll probably want to capitalize Student. Your writeto method should be declared as



void writeto(ostream &datastore) const {


In particular, it shouldn't accept a, rather using this to identify the student; and it shouldn't open the file itself. It should be able to write to any stream (including cout).



For code like this:



char ch; cin >> ch;


Try to avoid adding multiple statements on one line, for legibility's sake.



A large section of your addrecord method should be moved to a method on Student. Specifically, this:



strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);


can be made into a method perhaps called initFromPrompt that initializes the current Student instance. Also, your "container" input pattern doesn't need to exist. For instance, for grade, simply



cin >> strnumber.grade;


This:



main();//restarts the process;


is misdesigned. You definitely don't want to be calling main yourself, for many reasons - including recursion. If you want to "restart the process", you should make some kind of top-level loop.



This:



ifstream myfile ("datastore.txt");//opens the file;
if (myfile.is_open())


is somewhat of an anti-pattern. This kind of code is going to lead to silent errors. Instead, read about how to enable std::ios::failbit so that you get exceptions from bad stream calls.






share|improve this answer









$endgroup$













  • $begingroup$
    Can you give me an example of how I would call the write to method after your edit? I'm having a hard time understanding how it would work
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:51






  • 1




    $begingroup$
    strnumber.writeto(datastore), where datastore is your opened file and strnumber is your student instance
    $endgroup$
    – Reinderien
    Dec 29 '18 at 23:40














4












4








4





$begingroup$

These comments:



//directs you based on what you are trying to do;


don't need to end in semicolons. You're speaking English rather than C++ inside of them.




astericks




is spelled asterisks.



This code:



int number = 0;
string strnumber = to_string(number);


seems out-of-place. If it's just test code, delete it.



struct student


First, you'll probably want to capitalize Student. Your writeto method should be declared as



void writeto(ostream &datastore) const {


In particular, it shouldn't accept a, rather using this to identify the student; and it shouldn't open the file itself. It should be able to write to any stream (including cout).



For code like this:



char ch; cin >> ch;


Try to avoid adding multiple statements on one line, for legibility's sake.



A large section of your addrecord method should be moved to a method on Student. Specifically, this:



strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);


can be made into a method perhaps called initFromPrompt that initializes the current Student instance. Also, your "container" input pattern doesn't need to exist. For instance, for grade, simply



cin >> strnumber.grade;


This:



main();//restarts the process;


is misdesigned. You definitely don't want to be calling main yourself, for many reasons - including recursion. If you want to "restart the process", you should make some kind of top-level loop.



This:



ifstream myfile ("datastore.txt");//opens the file;
if (myfile.is_open())


is somewhat of an anti-pattern. This kind of code is going to lead to silent errors. Instead, read about how to enable std::ios::failbit so that you get exceptions from bad stream calls.






share|improve this answer









$endgroup$



These comments:



//directs you based on what you are trying to do;


don't need to end in semicolons. You're speaking English rather than C++ inside of them.




astericks




is spelled asterisks.



This code:



int number = 0;
string strnumber = to_string(number);


seems out-of-place. If it's just test code, delete it.



struct student


First, you'll probably want to capitalize Student. Your writeto method should be declared as



void writeto(ostream &datastore) const {


In particular, it shouldn't accept a, rather using this to identify the student; and it shouldn't open the file itself. It should be able to write to any stream (including cout).



For code like this:



char ch; cin >> ch;


Try to avoid adding multiple statements on one line, for legibility's sake.



A large section of your addrecord method should be moved to a method on Student. Specifically, this:



strnumber.number = entrynumb();//grabs the proper student number;

string strcontainer;//to store string responses such as firstname/lastname;
int intcontainer;//to store int responses such as age, dob, etc;

cout << "First name? " << 'n';
cin >> strcontainer; strnumber.firstname = strcontainer;
spacer(1);
cout << "last name ? " << 'n';
cin >> strcontainer; strnumber.lastname = strcontainer;
spacer(1);
cout << "birthday ? " << 'n';
cin >> strcontainer; strnumber.birthday = strcontainer;
spacer(1);
cout << "age ? " << 'n';
cin >> intcontainer; strnumber.age = intcontainer;
spacer(1);
cout << "grade ? " << 'n';
cin >> intcontainer; strnumber.grade = intcontainer;
spacer(1);


can be made into a method perhaps called initFromPrompt that initializes the current Student instance. Also, your "container" input pattern doesn't need to exist. For instance, for grade, simply



cin >> strnumber.grade;


This:



main();//restarts the process;


is misdesigned. You definitely don't want to be calling main yourself, for many reasons - including recursion. If you want to "restart the process", you should make some kind of top-level loop.



This:



ifstream myfile ("datastore.txt");//opens the file;
if (myfile.is_open())


is somewhat of an anti-pattern. This kind of code is going to lead to silent errors. Instead, read about how to enable std::ios::failbit so that you get exceptions from bad stream calls.







share|improve this answer












share|improve this answer



share|improve this answer










answered Dec 29 '18 at 3:32









ReinderienReinderien

4,180822




4,180822












  • $begingroup$
    Can you give me an example of how I would call the write to method after your edit? I'm having a hard time understanding how it would work
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:51






  • 1




    $begingroup$
    strnumber.writeto(datastore), where datastore is your opened file and strnumber is your student instance
    $endgroup$
    – Reinderien
    Dec 29 '18 at 23:40


















  • $begingroup$
    Can you give me an example of how I would call the write to method after your edit? I'm having a hard time understanding how it would work
    $endgroup$
    – Vildjharta
    Dec 29 '18 at 19:51






  • 1




    $begingroup$
    strnumber.writeto(datastore), where datastore is your opened file and strnumber is your student instance
    $endgroup$
    – Reinderien
    Dec 29 '18 at 23:40
















$begingroup$
Can you give me an example of how I would call the write to method after your edit? I'm having a hard time understanding how it would work
$endgroup$
– Vildjharta
Dec 29 '18 at 19:51




$begingroup$
Can you give me an example of how I would call the write to method after your edit? I'm having a hard time understanding how it would work
$endgroup$
– Vildjharta
Dec 29 '18 at 19:51




1




1




$begingroup$
strnumber.writeto(datastore), where datastore is your opened file and strnumber is your student instance
$endgroup$
– Reinderien
Dec 29 '18 at 23:40




$begingroup$
strnumber.writeto(datastore), where datastore is your opened file and strnumber is your student instance
$endgroup$
– Reinderien
Dec 29 '18 at 23:40


















draft saved

draft discarded




















































Thanks for contributing an answer to Code Review Stack Exchange!


  • Please be sure to answer the question. Provide details and share your research!

But avoid



  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.


Use MathJax to format equations. MathJax reference.


To learn more, see our tips on writing great answers.




draft saved


draft discarded














StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f210535%2ftext-based-student-information-system%23new-answer', 'question_page');
}
);

Post as a guest















Required, but never shown





















































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown

































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown







Popular posts from this blog

Bressuire

Cabo Verde

Gyllenstierna