A terrible function - or why I suck at programming.

18/1/2016

Now, I don't claim to be good at programming, in fact I know I'm not. I have been self-taught, there was no such thing as a programming class or course at my school or surrounding area, which sucked.
But today something different happened.
Today I found myself writing a function for my RGB controller project. I had decided to add a 'presets' feature to my project, allowing one to create RGB values they enjoy and save them to file for later use. This idea was great, so I began.
Twenty or so minutes later I was done. Done, yet highly disappointed.
Here is my controller now with the presets feature added. It is pretty useful.

However, the point of this post is not to display new features, instead it is to come out about the shame I feel for have written a particular function that is ugly, dangerous and messy.
The function in question is displayed below and is one that is used to read the presets file that contain information to be displayed in the application.

 
void MainWindow::readPresets() // read presets from file and display them
{
    DebugLog("Reading presets from file");
    // read file if it exists, display on screen
    QFile file("/home/daniel_j/presets.cfg"); // change at some point!
    if (file.exists()) {
        file.open(QIODevice::ReadOnly | QIODevice::Text);
        QTextStream in(&file);
        while(!in.atEnd())
        {
            QString line = in.readLine();
            QStringList presetName;
            if (line.contains(":"))
            {
                presetName = line.split(":");
                ui->presetList->addItem(presetName[0] + "-" + presetName[1]);
            } else {
                DebugLog("Error parsing line: " + line);
            }
        }
    } else {
        DebugLog("Presets file does not exist");
    }
}
 

There are many issues, both dangerous and cosmetic in this function that I'd like to point out.
The first, and most dangerous is my use of line.split() and line.contains().
To put it simply, at the time I was facing "index out of range" errors when attempting to split the line retrieved from the presets file. So, the solution, I thought was simple. Check that the line contains the character I use as the character I will split later in the function, and it works. Great! But no, not at all. Later in this function I use the split function to split the line into two and store it in a 'QStringList'. Once again, it all looks like it is going to work, and it does.The issue arises later in a function that sends the RGB value to my Arduino, at this point, the program would crash and close fatally. The issue is only occurs when a user manually edits the presets file and finishes the line with the character I use to split, ':'.
The solution that I need to, and will implement is proper line checking, instead of simple contains() checks.
The final issue I will touch upon (this post is already too long) is that of my use of QFile, specifically the file location I use.
The line in question is 'QFile file("/home/daniel_j/presets.cfg"); // change at some point!'.
Ahem, let me clear my throat, NEVER HARD CODE FILE PATHS THAT ARE NOT UNIVERSAL.