Tips/Improvement: File Parsing Program

[oresund]

I don't know if "parsing" is the right word, but this program is designed to get an input file "playlist.mbp" (musicbee playlist) and generate a simple M3U playlist file from it. I'd like any feedback on things I should've done differently and ways to make it more efficient, please.

Input file example: http://dpaste.com/27QZSKX
I used that ÿ character as a delimiter.

 int main(int argc, char* argv[])
{
	if (argc == 2){
		// change file extension
		string outputName(argv[1]);
		outputName.replace(outputName.length() - 3, 3, "m3u");
		//open input and output files
		ifstream inputMbp (argv[1], ios::binary);
		assert(inputMbp.is_open());
		ofstream outputM3u(outputName);
		assert(outputM3u.is_open());

		//put file contents into string and erase junk at beginning
		string fileContents = "";
		getline(inputMbp, fileContents);
		fileContents.erase(0, 72); 

		//write the piece that we need then erase it, until nothing left
		while (fileContents.find_first_of("ÿ") != string::npos){
			int k = fileContents.find_first_of("ÿ"); //called function twice
			outputM3u << fileContents.substr(0, k) << endl;
			fileContents.erase(0, k + 5);
		}
	}
	else cout << "Usage:\n mbp2m3u \"<input file path>\"\n";
	return 0;
}

[shadowmouse]

Why the if argc == 2? And for that matter why do so many people put the int argc, char* argv[] in main when you don't need to?

[oresund]

Why the if argc == 2?

The usage is <program name> <file path> which is 2 arguments. How else should I have done that?

[novelloff]

@shadowmouse

If you compile and run your program through the command prompt or terminal window you can pass in parameters to the main function. argc stands for argument count. The first argument being the command used to run the program,which is the program name, the second argument being the argument that is being passed to main function.

[oresund]

Are there any ways to call find_first_of() only once?

[andywestken]

I don't know the MusicBee playlist file format (have you seen a spec for it?), but it disquiets me slightly to see a binary file (or at least, one opened as binary) read into a std::string using std::getline. Might it be safer to work with a char buffer instead?

Anyway...

Are there any ways to call find_first_of() only once?

Yep.

For example:

#include <iostream>
#include <string>
using namespace std;

int main() {
    string story = "once    upon    a    time    ";

    size_t pos = string::npos;
    while (string::npos != (pos = story.find_first_of(" "))){
        cout << "\"" << story.substr(0, pos) << "\"" << endl;
        story.erase(0, pos + 4);
    }

    return 0;
}

or this non-destructive version (string now constant):

#include <iostream>
#include <string>
using namespace std;

int main() {
    const string story = "once    upon    a    time    ";

    size_t from = 0;
    size_t to   = string::npos;
    while (string::npos != (to = story.find_first_of(" ", from))){
        cout << "\"" << story.substr(from, to - from) << "\"" << endl;
        if(to != string::npos)
            from = to + 4;
    }

    return 0;
}

Andy

PS Given the size of the file I don't think efficiency is an issue. But it might be better to replace the asserts with code which reports the errors to the use in a more friendly manner? And inverting the if (argc == 2){ logic to the usage message is at the top of main might make it more obvious.