7
\$\begingroup\$

I am attempting to teach myself C++ by writing a simple multi-line text reader that reports the lengths of each line. What can I do better now that I have something that "works?"

Conventions:

  • variables and members are declared const, if I found it possible
  • variables and private members are written in camelCase
  • private members are prefixed with m_
  • public members are written in PascalCase

Please note that I will generally ignore comments regarding conventions because the style used is something I conform to in all of my personal projects, to maintain some sort of consistency across languages. The advice is still greatly appreciated for the times where I might be working on a team that is used to a more idiomatic C++ style.

Code:

#include <errno.h>
#include <stdbool.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

#define DATA_BUFFER_SIZE 4096
#define DELIMIT_CHAR ','
#define QUOTE_CHAR '"'
#define SOURCE_FILE_PATH "..."

struct FileBuffer {
    public:
        char* const BufferData;
        size_t const BufferLength;
        FILE* const FilePointer;
        size_t NumBytesRead;
        size_t NumBytesUsed;

        FileBuffer(char* const bufferData, size_t bufferLength, FILE* const filePointer) : BufferData(bufferData), BufferLength(bufferLength), FilePointer(filePointer), NumBytesRead(0), NumBytesUsed(0) { }
};

struct LineReader {
    private:
        FileBuffer* const m_fileBuffer;
        char const m_quoteChar;

        bool FillBuffer() const {
            return (m_fileBuffer->NumBytesUsed = 0, m_fileBuffer->NumBytesRead = fread(m_fileBuffer->BufferData, 1, m_fileBuffer->BufferLength, m_fileBuffer->FilePointer));
        }
        char NextChar() const {
            if ((m_fileBuffer->NumBytesUsed < m_fileBuffer->NumBytesRead) || FillBuffer()) {
                return m_fileBuffer->BufferData[m_fileBuffer->NumBytesUsed];
            }
            else {
                return '\0';
            }
        }

    public:
        LineReader(FileBuffer* const fileBuffer, char const quoteChar) : m_fileBuffer(fileBuffer), m_quoteChar(quoteChar) { }

        int operator()() const {
            FileBuffer* const fileBuffer = m_fileBuffer;
            const char* const BufferData = fileBuffer->BufferData;
            bool isQuotedSequence = false;
            int lineLength = 0;
            char const quoteChar = m_quoteChar;

            while ((fileBuffer->NumBytesUsed < fileBuffer->NumBytesRead) || FillBuffer()) {
                char const currentChar = BufferData[fileBuffer->NumBytesUsed++];
                lineLength++;

                if (quoteChar == currentChar) {
                    if (quoteChar == NextChar()) {
                        lineLength++;
                        fileBuffer->NumBytesUsed++;
                    }
                    else {
                        isQuotedSequence = !isQuotedSequence;
                    }
                }

                if (isQuotedSequence) {
                    continue;
                }

                if ('\n' == currentChar) {
                    return lineLength;
                }

                if ('\r' == currentChar) {
                    if ('\n' == NextChar()) {
                        lineLength++;
                        fileBuffer->NumBytesUsed++;
                    }

                    return lineLength;
                }
            }

            return ((lineLength == 0) ? -1 : lineLength);
        }
};

int main() {
    char dataBuffer[DATA_BUFFER_SIZE];
    const char* const fileName = SOURCE_FILE_PATH;
    FILE* filePointer;

    if (NULL != (filePointer = fopen(fileName, "r"))) {
        FileBuffer fileBuffer = FileBuffer(dataBuffer, DATA_BUFFER_SIZE, filePointer);
        LineReader const lineReader = LineReader(&fileBuffer, QUOTE_CHAR);
        int result;
        int totalLength = 0;

        while (-1 != (result = lineReader())) {
            fprintf(stdout, "Length: %d\n", result);
            totalLength += result;
        }

        fclose(filePointer);
        fprintf(stdout, "Total Length: %d\n", totalLength);
    }
    else {
        fprintf(stderr, "cannot open file '%s': %s\n", fileName, strerror(errno));
    }

    printf("Press ENTER key to continue...\n");
    getchar();

    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ There are a lot of things to improve. It is great that you came here! \$\endgroup\$ – Incomputable Feb 24 at 18:57
  • \$\begingroup\$ @Incomputable Yeah, coming from a C#/Java/SQL background, I know more than enough to brute force my way through most errors that the compiler throws at me; am certain that I'm violating all kinds of best practices. Problem I'm running into with C and C++ is that their history is so rich... there is all kinds of conflicting advice out there. \$\endgroup\$ – Kittoes0124 Feb 24 at 19:00
3
\$\begingroup\$

What can I do better now that I have something that "works?"

Great, since your goal is to learn C++, now it is time to replace all the C features with C++ versions. Have a look at the following headers:

Additionally the following features are preferred in C++:

  • nullptr over NULL
  • constexpr over #define
  • reference might be better then pointer since references can't be null.

Resource management

Think about how your struct/class behave when copied/moved. Start doing resource management with RAII idom. What happens when two copies of FileBuffer end up pointing to the same buffer? Does FileBuffer need to store an external buffer or should the buffer be part of the struct?

\$\endgroup\$
3
\$\begingroup\$

The first thing you need to do is you need to know and understand two things. They are related but different.

  • You want to know how your file is structured for reading in its contents.
  • You want to know how your data structure(s) (classes, structs, variables in a namespace, etc.) are designed.

Knowing these two things will help you in parsing any kind of file either it be in text or binary format.

In your case you had explicitly stated:

Multi-Line text reader

As the name of your question suggests.


What you want to do next is just as user: wooooooooosh had stated. You want to remove all the C library functions from your code and use the C++ libraries instead.

When you write your code, you want to keep the file handling opening, reading contents to some buffer and closing separate from any parsing and or data manipulation. All you want your file handler to do is to extract and read the data.

Now there are several ways to write a function that will open a file if it exists and to read its contents and by two types (text & binary) but we will only focus on text here.

  • You can read a single character from the file.
  • You can read a single string up to the first white space you encounter.
  • You can read a single line.
  • You can do all of the above until no more can be read in.
  • You can read all of the contents in one go into a large single buffer; depending on the size of the file.

In your specific case, you stated a multi line text file so we will use that as a demonstration.

  • First you want to create a structure that will hold many lines from the file. We can do this with:
#include <vector>
#include <string>

std::vector<std::string> linesFromFile;
  • Next we need a file name; this is simple enough:
#include <string>

// If you know the name of the file and it is hard coded in the application:
// then it is best to make this `std::string` `const` as it can not be modified.
const std::string filename( "somefile.txt" );
  • Or we can ask the user for a file name:
#include <string>
#include <iostream>

int main() {
    std::cout << "Enter the file's name for reading\n";

    // In this case the user will enter the filename so we don't want it `const`.
    std::string filename;
    std::cin >> filename;  // Now you have to be careful with cin and getline and mixing the two;  
                           // I will not go into the details here but a google search would do the job for you.
    return 0;
}

However, none of this will work without a filestream object. There are 3 main types:

  • std::fstream - basic filestream can be read from and or written to based on the flags set when opening
  • std::ifstream - a type of filestream specifically for reading contents in
  • std::ofstream - a type of filestream specifically for writing contents to

In your case you have two options to read your file:

#include <fstream>
#include <string>

int main() {
    std::string filename( "somefile.txt" );
    std::fstream file( filename, std::ios::in ); // This will try to open file;

    // Or:

    std::fstream file2;

    file2.open( filename, std::ios::in ); 

    return 0;
}

The flag option tells fstream that we want to read contents from the file as fstream is a two way stream operation on files.

If you do not want to write to the file as it will be read only then you can do the following:

#include <string>
#include <ifstream> // <fstream> should work here too

int main() {
    std::string filename( "somefile.txt" );

    std::ifstream inFile( filename ); // this will try to open the file if it exists.

    // or
    std::ifstream inFile2;

    inFile2( filename );

    return 0;
}

Now that we have a file handle, a filename, and a container to hold our contents we can put them together. Instead I'll create a class to handle our data. Then write a function to open the file and save the contents to...

#include <exception>
#include <fstream>
#include <iostream>
#include <sstream>
#include <string>
#include <vector>

class FileData {
private:
    std::vector<std::string> fileContents_;
    std::size_t linesInFile_ { 0 };
public:
    FileData() = default; // empty base constructor
    explicit FileData( std::vector<std::string>& contents ) : linesInFile( contents.size() ) {}

    void addData( std::vector<std::string>& contents ) {
        fileContents = contents;
        linesInFile_ = contents.size();
    }

    std::vector<std::string>& getFileContents() { return fileContents_; }
    std::size_t numberOfLinesInFile() const { return linesInFile_; }
};

void getAllLinesFromFile(const char* filename, std::vector<std::string>& output) {
    std::ifstream file(filename);
    if (!file) {
        std::stringstream stream;
        stream << "failed to open file " << filename << '\n';
        throw std::runtime_error(stream.str());
    }

    std::string line;
    while (std::getline(file, line)) {
        if (line.size() > 0)
            output.push_back(line);
    }
    file.close();
}

int main() {
    // my function allows you to do this a couple of ways:
    std::vector<std:string> fileContentsA, fileContentsB;
    // Just by passing the name directly in as a const string literal.
    getAllLinesFromFile( "somefile.txt", fileConentsA );

    // Or by
    std::string filename( "somefile2.txt" );
    getAllLinesFromFile( filename.c_str(), fileContentsB ); 

    return 0;
}

The above will give us our vector of strings that we need; however we have to parse that for our data structure, but since our data structure's constructor and or add function matches directly as the pattern of the file contents all we need to do is constructor our class object and in this case there is no need to parse the file.

Now if you were reading individual words from a single line of text where specific words had specific meanings and other characters were values, then you would have to write a parser function to break the strings down into individual tokens. However, here is not the case it is a simple read and pass to object operation.

int main() {
    // my function allows you to do this a couple of ways:
    std::vector<std:string> fileContents

    std::string filename( "somefile.txt" );
    getAllLinesFromFile( filename.c_str(), fileContents );

    FileData myData( fileContents );  

    // Or

    FileData myData2;
    myData2.addData( fileContents );

    return 0; 
}

Here I have a default empty constructor that will later on require the use of the addData function, but I also have a constructor that will take the data directly. This will give the user flexibility when using the class. If you already have the data ready by the time you are creating the class object you can use the constructor and pass the data in. In some cases you may not be ready for the data, but you have to have the object first to be populated later.


One thing to take note of is that my file reading function is throwing and error message so in our main function we need to wrap it into a try catch block. The entire app would look something like this:

#include <exception>
#include <fstream>
#include <iostream>
#include <sstream>
#include <string>
#include <vector>

class FileData {
private:
    std::vector<std::string> fileContents_;
    std::size_t linesInFile_ { 0 };
public:
    FileData() = default; // empty base constructor
    explicit FileData( std::vector<std::string>& contents ) : linesInFile( contents.size() ) {}

    void addData( std::vector<std::string>& contents ) {
        fileContents = contents;
        linesInFile_ = contents.size();
    }

    std::vector<std::string>& getFileContents() { return fileContents_; }
    std::size_t numberOfLinesInFile() const { return linesInFile_; }
};

void getAllLinesFromFile(const char* filename, std::vector<std::string>& output) {
    std::ifstream file(filename);
    if (!file) {
        std::stringstream stream;
        stream << "failed to open file " << filename << '\n';
        throw std::runtime_error(stream.str());
    }

    std::string line;
    while (std::getline(file, line)) {
        if (line.size() > 0)
            output.push_back(line);
    }
    file.close();
}

int main() {
    try {

        std::vector<std:string> fileContents;
        getAllLinesFromFile( "somefile.txt", fileConents );
        FileData myFileData( fileContents );

    } catch( std::exception& e ) {
        std::cout << "Exception Thrown: " << e.what() << std::endl;
        return EXIT_FAILURE;
    }   
    return EXIT_SUCCESS;
}

The above works as intended yet there still may be room for improvement, and with C++20 around the corner; there will be many nice new features to learn; especially with the new filesystem.


-Edit- A user had left a comment stating that unfortunately that this is not a multi line reader. Yes this will not read multiple lines at a time, but it will read a single line of text and store it into a string and for each line of text those strings will be stored into a vector. This was a demonstration only to show the OP how to transition code from C to C++. They can take what I have given them and try to apply their own algorithm to fit their needs.

\$\endgroup\$
  • \$\begingroup\$ Unfortunatly, this isn't a valid multi-line reader. \$\endgroup\$ – Kittoes0124 Feb 25 at 14:26
  • \$\begingroup\$ @Kittoes0124 It may not be; but it is something that will read each line of code until the entire file has been read. Each line will be stored as a string and the vector of strings would be the entire file. I was demonstrating to the OP what a C++ application would look like compared to a C application. It is up to the OP to take it from here. \$\endgroup\$ – Francis Cugler Feb 26 at 0:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.