Revision on failed assignment

[gcampton]

This is a kinda long post, and I didn't want to post in beginners as I felt it didn't deserve the attention. This is purely for critiques or advice I guess.

Back at the end of last semester (Dec 02 2009) I was asked to write an assignment to take in a file with random strings, put it in an array of 100 elements, no duplicate strings, sort the array, then output to another file. Program was to make use command line arguments only accepting arguments in the format of: -i inputfile -o outputfile

Question: what if the array fills up.?
Answer: stop processing file and continue.

Anyway my result for that was bad, but due to our teacher leaving halfway through the semester due to a heartattack, getting a new teacher. and the date on the assignment being put back to Feb2010. I actually passed because I was only 1 of 3 who handed in their assignments early to get extra credit. I was dissapointed I did so badly, but in hindsight was doing a java and bourne scripting assignment at the same time. So I didn't quite test the assignment fully.

Anyway after 2 months of posting on these forums the revision only took me close to 40mins :D
All criticism is welcome, I'm curious how others would have gone about this. While still on about the same skill level.

While I would have liked to just stick it in the one file, I kept it in the format the teacher expected just for nostalgia:

SSMain.cpp

// main for String Sort.

#include "StringSort.h"

int main(int argc,char *argv[])
{
    StringSort ss;
    ss.checkArgs(argc,argv);
    ss.processInFile();
    ss.sortStrings();
    ss.processOutFile();
    
    return 0;
}

StringSort.h

#ifndef STRINGSORT_H_
#define STRINGSORT_H_

#include <iostream>
#include <string>
#include <cstdlib>
#include <fstream>

using std::cout;
using std::cerr;
using std::ifstream;
using std::ofstream;
using std::exit;
using std::string;
using std::endl;

class StringSort
{
public:
    void checkArgs(int argc,char *argv[]);
    string getInFile();
    string getOutFile();
    void processInFile();
    void processOutFile();
    void sortStrings();
    
private:
    void setInFile(string inputFileName);
    void setOutFile(string outputFileName);
    string inFile, outFile;
    const static int ARRAY_SIZE=100;
    string sSArray[ARRAY_SIZE];
};

#endif /*STRINGSORT_H_*/ 

StringSort.cpp

#include "StringSort.h"

void StringSort::setInFile(string inputFileName)
{
    inFile = inputFileName;
}

string StringSort::getInFile()
{
    return inFile;
}

void StringSort::setOutFile(string outputFileName)
{
    outFile = outputFileName;
}

string StringSort::getOutFile()
{
    return outFile;
}
    
void StringSort::checkArgs(int argc,char *argv[])
{
    int row=1;
    string sw;
    string inputFile="";
    string outputFile="";
    string usage="Usage: -i input_filename -o output_filename";
    
    while (row < argc)
    {
        sw = argv[row];
        if (sw == "-i")
        {
            row++;
            if (row < argc)
                inputFile = argv[row];
        }
        else if (sw == "-o")
        {
            row++;
            if (row < argc)
                outputFile = argv[row];
        }
        else
        {
            cerr << "Invalid switch: " << sw << "\n";
            cerr << usage << endl;
            exit (1);
        }
        row++;
    }
    
    if (inputFile.size() < 1)
    {
        cerr << "Error input file needs to be given!\n";
        cerr << usage << endl;
        exit (1);
    }
    else if (outputFile.size() < 1)
    {
        cerr << "Error: output file needs to be given!\n";
        cerr << usage << endl;
        exit (1);
    }
    if (inputFile == outputFile)
    {
        cerr << "Error: duplicate filenames, filenames must be unique!\n";
        cerr << usage << endl;
        exit (1);
    }
    else
    {
        setInFile(inputFile);
        setOutFile(outputFile);
    }
}

void StringSort::processInFile()
{
    ifstream ifstr(getInFile().c_str(), ifstream::in);
    string line;
    bool lineExists=false;
    int count=0;
    
    if (! ifstr.is_open())
    {
        cerr << "Error: unable to open file: " << getInFile() << "\n"
            << "Make sure file names are correct and files exist.\n"
            << "Usage: -i input_filename -o output_filename" << endl;
        exit (1);
    }
    if (! ifstr.good())
    {
        cerr << "Error: Bad input stream in: " << getInFile() << "\n"
            << "Check file is not corrupted and/or has correct permissions.\n"
            << "Usage: -i input_filename -o output_filename" << endl;
        exit (1);
    }
    
    while ((!ifstr.eof()) && (count<ARRAY_SIZE))
    {
        getline(ifstr, line);
        // check if line exists already
        for (int i=0; i<(count+1); i++)
        {
            if (line == sSArray[i])
                lineExists = true;
        }
        // if line doesn't exist add it to the array
        if (! lineExists)
            sSArray[count++] = line;
        
        // reset bool
        lineExists = false;
    }
    ifstr.close();
}

void StringSort::processOutFile()
{
    ofstream ofstr(getOutFile().c_str(), ofstream::out);
    if (! ofstr.is_open())
    {
        cerr << "Error: unable to open file: " << getOutFile() << "\n"
            << "Make sure file names are correct and files exist.\n"
            << "Usage: -i input_filename -o output_filename" << endl;
        exit (1);
    }
    if (! ofstr.good())
    {
        cerr << "Error: Bad input stream in: " << getOutFile() << "\n"
            << "Check file is not corrupted and/or has correct permissions.\n"
            << "Usage: -i input_filename -o output_filename" << endl;
        exit (1);
    }
    
    for (int i=0; i<ARRAY_SIZE; i++)
    {
        // don't store null strings
        if (! sSArray[i].empty())
            ofstr << sSArray[i] << "\n";
    }
    cout << endl;
    ofstr.close();
}

void StringSort::sortStrings()
{
    int i, j, min;
    
    for (i=0; i<(ARRAY_SIZE-1); i++)
    {
        min = i;
        for (j=(i+1); j<ARRAY_SIZE;j++)
        {
            if (sSArray[j] < sSArray[min])
                min = j;
        }
        if (i != min)
        {
            string swap = sSArray[i];
            sSArray[i] = sSArray[min];
            sSArray[min] = swap;
        }
    }
}

[gcampton]

My original had a bunch of errors with the way the loop functioned was in fact not stopping once the array was full. My error checking was really bad, like if you put in zero length filenames it would throw an error but the error was filenames are duplicates. Also my sort was bad.... TBH I haven't tested the above code very well, I ran 2 tests on it, one 6 lines of text, second test was 110 lines of text with last line being zzzz, just to make sure I learned from my mistakes :P

[moorecm]

I gave it a quick glance over and I must say it looks pretty solid. Good work.

What did it look like before? :P

[helios]

What was wrong with std::vector and std::sort()?

[jsmith]

What was wrong with std::set?

[gcampton]

moorecm wrote:

What did it look like before? :P

much much worse, actually my functions were pretty much the same, just written really badly.

helios wrote:

What was wrong with std::vector and std::sort()?

my instructor...

jsmith wrote:

What was wrong with std::set?

same as above ^


I hate the way he goes about things it's confusing and his reasoning as to why makes no sense. I asked him if over the holidays I should study linked, lists/vectors/templates. His answer was no, there's no need as we don't use them ever. I asked why? he says because he's only teaching us a basic language and if we stick the what he teaches us then the progression into C is very simple.

Personally I fail to see how, std::string, inheritance/classes was magically included in C without my knowledge, but whatever....

For the last 2 months I've had to just sit back and ignore posts that deal with vectors /templates etc. For my most part now I know how they work anyway, just from reading so many darn posts...