6
\$\begingroup\$

Using file stream object, write a program to read an external text file ("myCourse.txt") which contains all the course name and numbers you are taking this semester, and to generate an analysis (total number of courses) into an external file ("myCourseSummay.txt").

This is a past question my teacher has been hinting at us to practice, so my friend and I made this and wanted to know if would be acceptable. Personally, I think we messed the writing to the .txt file.

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

int intro()
{
    cout << "Hello World!";
    return 0;
}

int main()
{
    ofstream myCourse;
    myCourse.open("myCourse.txt");
    char course1[20], course2[20], course3[20], course4[20];

    cout << "Enter number of courses. max of 4" << endl;
    cout << "Enter 1st course: ";
    cin >> course1;
    myCourse << course1 << endl;
    cout << "Enter 2nd course: ";
    cin >> course2;
    myCourse << course2 << endl;
    cout << "Enter 3rd course: ";
    cin >> course3;
    myCourse << course3 << endl;
    cout << "Enter 4th course: ";
    cin >> course4;
    myCourse << course4 << endl;

    myCourse.close();

    ifstream myCourseSummary;
    string word;
    int count = 0;
    myCourseSummary.open("myCourseSummary.txt");

    while(!myCourseSummary.eof())
    {
        myCourseSummary >> word;
        count++;
    }

    cout << "Total number if courses is(are) " << count << "!";
    myCourseSummary.close();

    cin.get();
    return 0;
}
\$\endgroup\$
5
\$\begingroup\$

eof inside a loop condition

See why is eof inside a loop condition considered wrong? You do not want to do:

while (!myCourseSummary.eof())
{
    myCourseSummary >> word;
    count++;
}

You do want to do:

while (myCourseSummary >> word)
{
    count++;
}

Arbitrary length course names

You are reading coureses into a char[20]. What if I have a 25-character course name? What about 50? What if it has a space in it? You need to handle all of these conditions, so you should prefer to std::getline a std::string.

Actually solve the problem

The problem statement is:

read an external text file (“myCourse.txt”) which contains all the course name and numbers you are taking this semester

But you are prompting the user for course names and writing them to a file. You should read the file directly.

Also:

generate an analysis (total number of courses) into an external file (“myCourseSummay.txt”)

You are currently writing into "myCourse.txt" and reading from "myCouresSummary.txt". The question asks you to read from the first, and write a number into the latter. That is:

std::ifstream myCourse("myCourse.txt");
// do some reading
// come up with what count is

std::ofstream myCourseSummary("myCourseSummary.txt");
myCourseSummary >> count;

You should be able to handle any number of courses. Just read from the file until it's done.

\$\endgroup\$
  • \$\begingroup\$ Oh my god. thank you. my bad for writing to course and reading from my course summary. but something is bugging me about the question in general. Why are we writing to somewhere but reading from somewhere else? I don't get it. what is even in my "myCouresSummary.txt" anyway. isn't it all the content in my "myCourse.txt" \$\endgroup\$ – David Oct 30 '15 at 5:15
  • \$\begingroup\$ are We assuming that a file exists with all the courses inside? then do we need to write? \$\endgroup\$ – David Oct 30 '15 at 5:18
  • \$\begingroup\$ @David Do you understand the problem? It asks you to read in courses from a file, and then write the total number of courses into a different file. \$\endgroup\$ – Barry Oct 30 '15 at 12:44
5
\$\begingroup\$

In addition to the problems already noted, your code seems to differ from the directions in failing to read the course numbers along with the names. The directions state:

...which contains all the course name and numbers you are taking this semester...

[emphasis added].

At a guess, the input format might look something like this:

Intro to Statistics
AM203
Data structures and Algorithms
CS305

It's also possible they might use some other delimiter between the course name and number, such as tab or vertical bar (or just about anything else that won't occur in the course name).

To count the correctly, it's usually easiest to start with a function that reads exactly one complete "unit" of input. By convention in C++, this is named operator>>, so code might look something like this:

struct course { 
    std::string name;
    std::string number;

    friend std::istream &operator>>(std::istream &is, course &c) { 
        std::getline(is, c.name);
        std::getline(is, c.number);
        return is;
    }
};

To count these, I'd rather use a standard algorithm specifically for counting, rather than writing my own code to do the job:

std::ifstream in("myCourse.txt");

std::cout << std::count(std::istream_iterator<course>(in), 
                        std::istream_iterator<course>());

For simplicity, I've written the result to cout, but you really want to open an ofstream, and write to that instead.

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

I see a number of things that may help you improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Eliminate unused functions

The intro() function is not used. Eliminating it makes the code easier to understand and maintain.

Read the requirements carefully

Your description of the program and what the program actually does don't really match. In particular, the description says to read "myCourse.txt" and write "myCourseSummary.txt" but the code does exactly the opposite.

Choose the appropriate loop end conditions

The file reading loop is currently this:

while(!myCourseSummary.eof())
{
    myCourseSummary >> word;
    count++;
}

There are a few different problems with this, but the most important one is that using eof() as a loop ending condition is almost always wrong. Instead, write it like this:

while (myCourseSummary >> word) {
    count++;
}

Separate things into functions

The code repeats itself a number of times and does everything in main. Instead, use a separate function to do repetitive things such as issue a prompt and read a course name. Then use the function.

Prefer std::string over char[]

What happens if somebody types more than 20 characters for a course name? Buffer overrun! Better would be to use a std::string rather than an array of char.

Don't number variables

Whenever you find yourself numbering variables such as course1, course2, etc., you should ask yourself if this shouldn't be an array or std::vector instead. In this case, it's very limiting to only be able to have 4 courses, and due to the way the code is structured, it would require a bit of tinkering to add more.

Create and open files

You can open and create a file using a constructor. For instance,

std::ofstream myCourse("myCourse.txt");

This makes the code more clear and doesn't separate the variable instantiation and file opening with a number of unrelated lines of code as with the ifstream declaration later in the code.

Isolate and name important constant strings

The file names are currently hardcoded strings embedded in the code. It could be that one may wish to change those (or at least identify them more easily) later. To facilitate this, I'd suggest naming them:

const std::string inFileName{"myCourse.txt"};

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.

\$\endgroup\$
  • \$\begingroup\$ I'm still new to this course and this is how my professor taught both of us. but i will take this advice to heart. thank you \$\endgroup\$ – David Oct 30 '15 at 5:32
  • \$\begingroup\$ Teaching often benefits from omitting some details until the student has acquired sufficient knowledge to understand them. This is likely the difference between what I've written and your professor's approach. \$\endgroup\$ – Edward Oct 30 '15 at 13:00

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.