Text based student information system












1














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









New contributor




Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.




















  • Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    – mickmackusa
    2 hours ago










  • 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.
    – Vildjharta
    2 hours ago










  • It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    – mickmackusa
    1 hour ago










  • 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.
    – Vildjharta
    1 hour ago
















1














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









New contributor




Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.




















  • Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    – mickmackusa
    2 hours ago










  • 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.
    – Vildjharta
    2 hours ago










  • It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    – mickmackusa
    1 hour ago










  • 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.
    – Vildjharta
    1 hour ago














1












1








1







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









New contributor




Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.











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









New contributor




Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.











share|improve this question









New contributor




Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









share|improve this question




share|improve this question








edited 1 hour ago









Reinderien

3,035720




3,035720






New contributor




Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.









asked 2 hours ago









Vildjharta

62




62




New contributor




Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.





New contributor





Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.






Vildjharta is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.












  • Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    – mickmackusa
    2 hours ago










  • 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.
    – Vildjharta
    2 hours ago










  • It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    – mickmackusa
    1 hour ago










  • 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.
    – Vildjharta
    1 hour ago


















  • Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
    – mickmackusa
    2 hours ago










  • 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.
    – Vildjharta
    2 hours ago










  • It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
    – mickmackusa
    1 hour ago










  • 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.
    – Vildjharta
    1 hour ago
















Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
– mickmackusa
2 hours ago




Is there a particular aspect that you are seeking refinement with? Improved performance? Brevity? Etc.
– mickmackusa
2 hours ago












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.
– Vildjharta
2 hours ago




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.
– Vildjharta
2 hours ago












It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
– mickmackusa
1 hour ago




It is important that you clarify in your question/title exactly how you would like your code to be reviewed.
– mickmackusa
1 hour ago












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.
– Vildjharta
1 hour ago




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.
– Vildjharta
1 hour ago










2 Answers
2






active

oldest

votes


















2














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





























    1














    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





















      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
      });


      }
      });






      Vildjharta is a new contributor. Be nice, and check out our Code of Conduct.










      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









      2














      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


























        2














        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
























          2












          2








          2






          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












          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 1 hour ago









          Reinderien

          3,035720




          3,035720

























              1














              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


























                1














                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
























                  1












                  1








                  1






                  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












                  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 49 mins ago









                  Edward

                  46.1k377209




                  46.1k377209






















                      Vildjharta is a new contributor. Be nice, and check out our Code of Conduct.










                      draft saved

                      draft discarded


















                      Vildjharta is a new contributor. Be nice, and check out our Code of Conduct.













                      Vildjharta is a new contributor. Be nice, and check out our Code of Conduct.












                      Vildjharta is a new contributor. Be nice, and check out our Code of Conduct.
















                      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.





                      Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


                      Please pay close attention to the following guidance:


                      • 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.


                      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

                      flock() on closed filehandle LOCK_FILE at /usr/bin/apt-mirror

                      Mangá

                      Eduardo VII do Reino Unido