Config Parser Suggestions?

[MaxterTheTurtle]

Hello, recently I wrote a small program that handles configuration files. I was wondering if this code is considered "good?" I am also wondering if there are ways to optimize it. I am open to suggestions, just be nice please. Also, don't mind the comments. I know that there are too many of them.

class configFile {
	std::vector<std::string> variables;
	std::vector<std::string> values;

public:
	/* declare the constructor function */
	configFile(std::string loc) { 

		/* declare an file input object to read from the file */
		std::ifstream file;

		/* open the file from the location sent in the function parameters */
		file.open(loc);

		/* declare a vector object to hold the contents of the object */
		std::vector<std::string> contents;

		/* determine if the file is open */
		if (file.is_open()) {

			/* read the file until the end of file */
			while (!file.eof()) { 

				/* declare a string to read the current line to */
				std::string line;

				/* read the current line from the file */
				getline(file, line);

				/* push the current line into the contents vector */
				contents.push_back(line);
		} }

		/* close the file */
		file.close();

		/* iterate through all of the contents vector */
		for (int i = 0; i < contents.size(); i++) {

			/* remove all of the spaces, tabs, and delete comments */
			while (contents[i].find(' ') != std::string::npos) { contents[i].erase(contents[i].find(' '), 1); }
			while (contents[i].find('\t') != std::string::npos) { contents[i].erase(contents[i].find('\t'), 1); }
			while (contents[i].find('#') != std::string::npos) { contents[i].erase(contents[i].find('#')); }

			/* remove the vector if the string is empty */
			if (contents[i].empty()) { contents.erase(contents.begin(), contents.begin()+1); }
		}

		/* iterate through all of the remaining strings */
		for (int i = 0; i < contents.size(); i++) {

			/* determine if an equal sign has been found on this line */
			if (contents[i].find('=') != std::string::npos) {

				/* determine if there is another equals sign on this line (illegal assignment operator) */
				if (contents[i].substr(contents[i].find('=')+1).find('=') != std::string::npos) {

					/* removethis vector, since there is an illegal assignment operator */
					contents.erase(contents.begin(), contents.begin()+1); continue;
				}

				/* assign the variable to the variable vector */
				variables.push_back(contents[i].substr(0, contents[i].find('=')));

				/* assign the value to the value vector */
				values.push_back(contents[i].substr(1+contents[i].find('=')));

			} else {
				/* remove this vector, since it doesn't have an equal sign */
				contents.erase(contents.begin(), contents.begin()+1);
		}	}
	}

	/* declare the function to find the variable and its value */
	std::string find(std::string variable) {
		/* iterate through each of the variables in the vector */
		for (int i = 0; i < variables.size(); i++) {
			/* determine if the variable is found */
			if (variables[i].compare(variable) == 0) {
				/* return the value of that variable */
				return values[i];
		}	}
		/* return void, since no value was found */
		return "void";
	}
};

Thanks,
Maxtertheturtle

[writetonsharma]

Most of the times there is no good or bad in programming. something good for one can be bad for others. ofcourse there are some universal good/bad practices. Below are some of my comments.

- when you write comments, just tell what is being done and don't tell how its been done.
- If you feel like writing lots of comments, revisit your code and check how you can minimize the comments by making your code easier to understand.
- Give the user of your class different options to create the object and not just one. He may not always have a path/filename to initialize the object.
- Don't think the person creating the config file is a robot. So,
variable=value
variable = value
variable =
etc are all correct.
A program should be easy to use and should complain rarely unless the program can not proceed at all. What you can do is, create tokens and then remove all the spaces, tabs etc.
- the larger a function is, the more difficult it will be to understand and find bugs in it. Make functions smaller and try to make sure that a function should do one kind of task. So, opening can be done in one function, reading the file in other, making tokens in other etc.. All these can be done using different public functions available to the user.
- rather than returning void, return blank strings when something is not found. Returning an error code is better. give proper error/log if some error/warning occurs. Otherwise, the user will scratch the head.

[MikeyBoy]

Also, don't mind the comments. I know that there are too many of them.

Believe me, it's much better to have too many comments, than too few. Obviously, there are times when it's so obvious what the code does that there's no need for a comment - e.g. your line 34 - but in general, if someone's going to err, I'd much rather they erred on the side of putting in too many comments :)

[MaxterTheTurtle]

Hello, I followed your advice writetonsharma. It was good, I never thought about the different ways that someone would use config file. I just assumed that someone would load the file when the class was created. Thanks for Pointing that out! I will also return a void string instead of "void".

Thanks!