mjhjmtu

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 

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';
 

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 resultdatastore;
 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.

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.