why is it skipping lines?

I have a function called addChecksum that is called from a switch statement. The function is to ask for a file path to a file, then to open the file. But if it doesn't open, then it should take you back to the menu. Right now, it runs through the entire function without giving me a chance to enter the file path. I don't understand why. Here is my code

Code:

---

#include "stdafx.h"
#include <string>
#include <iostream>
#include <iomanip>
#include <cmath>
#include <fstream>
using namespace std;

void selection();
void addChecksum();
void runChecksum();

int main()
{
        selection();
       
    system("pause");
    return 0;
}

void selection()
{
        char choice;

        cout << "Please Select: " << endl;
        cout << "  A) Add checksum to specified file" << endl;
        cout << "  B) Verify integrity of specified file" << endl;
        cout << "  Q) Quit" << endl;

        cin >> choice;
        cout << endl;

        switch (choice)
        {
        case 'A':
                addChecksum();
                break;
        case 'a':
                addChecksum();
                break;
        case 'B':
                runChecksum();
                break;
        case 'b':
                runChecksum();
                break;
        case 'Q':
                return;
        case 'q':
                return;

        default:
                cout << "Your choice did not match any of the menu options." << endl;
        }
}

void addChecksum()
{
        string inputFileName;
        ifstream inputFile;
        string line;

        cout << "Specify the file path: ";
        getline(cin, inputFileName);

        inputFile.open(inputFileName.c_str());
        cout << endl;

        while(!inputFile.is_open())
        {
                cout << "The file " << inputFileName << "could not be found or opened!" << endl;
                selection();
        }
}

void runChecksum()
{
        string inputFileName;
        ifstream inputFile;
        string line;
        int checksum;

        checksum = 0;

        while(!inputFile.is_open())
        {
                cout << "Specify the file path: " << inputFileName;
                getline(cin, inputFileName);

                inputFile.open(inputFileName.c_str());
        }

        cout << "File checksum = " << checksum << endl;
}

---

Because you need to clear the cin buffer of invalid characters:

Code:

---

#include <string>
#include <iostream>
#include <iomanip>
#include <cmath>
#include <fstream>
#include <cctype> // for tolower
#include <limits>  //for numeric_limits

using namespace std;

void selection();
void addChecksum();
void runChecksum();

int main()
{
    selection();
   
    // A more portable version of system("pause") is:
    std::cout << "Press return to continue" << std::endl;
    std::cin.get();
    return 0;
}

void selection()
{
    char choice;

    cout << "Please Select: " << endl;
    cout << "  A) Add checksum to specified file" << endl;
    cout << "  B) Verify integrity of specified file" << endl;
    cout << "  Q) Quit" << endl;

    cin >> choice;
    cout << endl;

   //Clear anything currently in the cin buffer
    std::cin.clear();
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');

  //You can reduce the size of your switch statement by using tolower.
    [COLOR=Red]switch (tolower(choice))
    {
    case 'a':
        addChecksum();
        break;
    case 'b':
        runChecksum();
        break;
    case 'q':
        return;
    default:
        cout << "Your choice did not match any of the menu options." << endl;
    }
}

void addChecksum()
{
    string inputFileName;
    ifstream inputFile;
    string line;

    cout << "Specify the file path: ";
    getline(cin, inputFileName);

    inputFile.open(inputFileName.c_str());
    cout << endl;

    while(!inputFile.is_open())
    {
        cout << "The file " << inputFileName << "could not be found or opened!" << endl;
        //You have a bug here. What if someone realises they don't
        //want to load a file anymore, but want to quit instead?
        //selection will keep being called even though the user is
        // pressing 'q' because they have not entered a valid file name!
        selection();
    }
}

void runChecksum()
{
    string inputFileName;
    ifstream inputFile;
    string line;
    int checksum;

    checksum = 0;

    while(!inputFile.is_open())
    {
        cout << "Specify the file path: " << inputFileName;
        getline(cin, inputFileName);

        inputFile.open(inputFileName.c_str());
    }

  //I guess you are going to do your calulation here?
    // ....
    cout << "File checksum = " << checksum << endl;
}

---

You shouldn't keep calling functions recursively like that in this case. Set up a while loop instead.

The mixing of normal cin with getline() is a known complication. You can deal with it, though. There are hundreds of articles talking about the situation and its solutions. Try googling "skipped getline".

The strange thing is that the code works just fine if I put it in a while loop. It does not skip the getline. So I googled it and added inputFile.clear(); but it still skips. I will keep googling. If you have another suggestion, I would be pleased to read it.

Which webpage is telling you to use .clear()?

I am reading a bunch of pages. Some say to clear the file stream, some say to use ignore, others have elaborate while statements that have stuff to do with using the /n. I am still looking for the solution

The key thing is to put an ignore between a normal cin and a getline----but not put one between two getlines. This can cause some difficulties if you're accessing the same stream in multiple functions.

The approach I usually take is to use getline() for every line, and then parse the line separately if necessary using a stringstream. That gets around the whole issue.

I have only been working with c++ for 5 weeks. I am researching ignore() and how to use it right now

Quote:

---

Originally Posted by CyberShot

The strange thing is that the code works just fine if I put it in a while loop.

---

The code you did post does the following:

selection calls addchecksum, which calls selection, which calls addchecksum, which calls selection, which calls addchecksum ...

A few of these, and your stack space gets exhausted. Imagine if your program was supposed to be run non-stop.

There is no need for recursion in such a simple program. So when you say "while loop", are you referring to the code you posted or different code?

Regards,

Paul McKenzie

I am following an assignment posted by my instructor. I had to change to code in order to call the menu again if the input file could not be found. Before that I had the code in addChecksum like this

Code:

---


        string inputFileName;
        ifstream inputFile;
        string line;

        while(!inputFile.is_open())
        {
              cout << "Specify the file path: ";
              getline(cin, inputFileName);
                inputFile.open(inputFileName.c_str());
              cout << endl;
        }

---

with the getline(cin, inputFileName) in the while loop, there was no problem, the code worked as it should. Once I took it out of the while loop in order to recalll the menu if the opening of the file failed, that is when I got the problems. I just fixed it though by placing

cin.ignore();

just before my getline();

So now it is working. Thanks Lindley for your help

You don't appear to be understanding Paul's objection. Your current code never allows any of these functions to return, so you just keep going deeper and deeper onto the stack. This will break things eventually.

In general, you should only be calling your menu from main(). You "get back to it" when necessary by returning from other functions to main in such a way that the program loops back to the selection() call.

I think I see what you are saying. I am doing it this way because I can easily understand what is going on at my level of programming. To do it your way would take me much more time than I have because I would have to figure it out. The program only needs to work once

Well, just don't learn the bad habit. Your next program should be designed to do things differently from the start.

Quote:

---

Originally Posted by CyberShot

I think I see what you are saying. I am doing it this way because I can easily understand what is going on at my level of programming. To do it your way would take me much more time than I have because I would have to figure it out. The program only needs to work once

---

Appearing to work and actually working are two different things. Unless you really understand recursion, its possible side effects and potential pitalls, you shouldn't be using it, especially when it isn't called for. In this case, its use is just plain wrong.

well, like I said, I have only been working with C++ for 5 weeks. So no, I don't fully understand anything about C++. What I do no is that so far the program works and is doing what I think it should do. At this stage in the game that is good enough. Maybe after a year or so I will be able to rewrite this program with all the things you guys are talking about.

Quote:

---

Originally Posted by CyberShot

I think I see what you are saying. I am doing it this way because I can easily understand what is going on at my level of programming.

---

And the ironic thing is that you didn't really know what you were doing until we pointed it out to you. You may still not understand what you really are doing with respect to recursion.

Quote:

---

To do it your way would take me much more time than I have because I would have to figure it out.

---

Well, isn't that what learning is about, to figure things out? In addition, if the teacher is awake, they should take points off if you handed this in -- programming isn't just getting input, calculating, and getting output. You have to make sure the program flow is correct, and using recursion is not correct.

Besides, it probably is easier to do this non-recursively, despite your claim of it being more difficult.

Quote:

---

The program only needs to work once

---

It doesn't work once. "Once" means that the program works from beginning to end. That is not the case with your code.

If I sat there at the keyboard and chose choice 'A' thousands of times without exiting, your program bombs out.

Regards,

Paul McKenzie

Code:

---

start program

do
{
    display menu and get selection
    if ( selection is 'a') do a stuff
    if ( selection is 'b') do b stuff
}  while (selection is not 'q') // meaning quit

end program

---

That is basically the pseuodocode you claim takes a lot of time to figure out.

Regards,

Paul McKenzie

I do understand most of what you are getting at. But when you are learning, you don't learn it by knowing everything. Like I said, I only have one week for this program and beyond that, there are lots of other things that I need to do for other classes. I can't spend the entire week on it. there will be error checking that I will add in there. Right now, this is just a skeleton of the program, a rough draft kind of thing.

what you want is for me to skip first, second and third base and go straight home. I don't have that much time.

Quote:

---

Originally Posted by CyberShot

what you want is for me to skip first, second and third base and go straight home. I don't have that much time.

---

The thing you are not understanding is that doing this the right way was easier than what you wrote. Look at my pseudo-code above.

Using recursion is "going to home plate before reaching first base", since most beginners do not learn recursive functions and its purpose until the middle or end of their first course.

Regards,

Paul McKenzie

and if you want an alternative:

Code:

---

start program

set selection to 'a'

while selection is not 'q'
{
    display menu and get selection
    if ( selection is 'a') do a stuff
    if ( selection is 'b') do b stuff
} 

end program

---

Regards,

Paul McKenzie

Quote:

---

Originally Posted by CyberShot

well, like I said, I have only been working with C++ for 5 weeks. So no, I don't fully understand anything about C++. What I do no is that so far the program works and is doing what I think it should do. At this stage in the game that is good enough. Maybe after a year or so I will be able to rewrite this program with all the things you guys are talking about.

---

You'll still need to learn to write iterative loops correctly. Even if this program appears to work, the next one won't. You've written code that's guaranteed to crash if you loop too many times. As you say, you don't fully understand C++, so take the advice of those of us a little further down the road. Learn how to use a while loop in cases like this. It really isn't optional.

I completely understand where you guys are coming from. The points you made are valid and I will look into improving my code so that it doesn't crash if you were to enter the choice 5 times then press enter. I have just tested it and I see how it crashes now.

My point is this. Doing all that work right now would not be beneficial to my learning. You ever notice that when you are reading a programming book, it will introduce you to a subject and then say something like "But more on that in a later chapter". I am on chapter 5, 6 and 7 here. I feel like a lot of what you guys are talking about are in chapters 20, 21 and so on. I can't skip ahead and design the program to be flawless. Right now, all my instructor requires is that the program works. It doesn't need to purr like a kitten, it just needs to produce the answer he is looking for. All the error checking will come into play in later chapters.

Your points are well taken and I will look to implement them if I can figure it out. I am researching the clear() and ignore() functions right now to see if they can clean things up a little.

Hmmm.....the issue they're worried about is more fundamental to how functions work. Not quite chapter 1, but....maybe chapter 2. It's appearing that you may have a fundamental misunderstanding of how functions work, and that needs to be corrected ASAP.

What you are trying to do is to return to the top of the selection() function if addChecksum fails. You are doing this, but in the wrong way----instead of returning control upward with an error indication, you're burrowing ever further downward.

I would agree with you on the fundamental misunderstandings of functions. We are using
C++ Programming From problem analysis to program design by D.S Malik. Functions are chapters 6 and 7. I am on chapter 7 right now which is User defined functions 2.

I will put some thought into what you guys have said and see if I can come up with another way to write this. Maybe my instructor will have some input, I will ask him on thursday when I attend class.

P.S When I was googling skipping getline, this post came up in the results. I thought it was funny, this site comes up quick in google

Quote:

---

Originally Posted by CyberShot

I do understand most of what you are getting at. But when you are learning, you don't learn it by knowing everything. Like I said, I only have one week for this program and beyond that, there are lots of other things that I need to do for other classes. I can't spend the entire week on it. there will be error checking that I will add in there. Right now, this is just a skeleton of the program, a rough draft kind of thing.

what you want is for me to skip first, second and third base and go straight home. I don't have that much time.

---

Did your instructor specifically tell you to solve the problem using recursion in this manner? If not, then what you're shooting for is probably going to be a bad grade, because you're using recursion incorrectly and have a program that's not able to execute successfully. What they're asking you to do is not high end or complicated stuff - basic flow control like a while loop should be about the second or 3rd thing taught in any decent programming class.

If they did tell you to do it this way... then that's another problem entirely. If that's the case you have to make them happy to get the grade, but understand that they're teaching you something totally wrong.

Quote:

---

Originally Posted by CyberShot

"But more on that in a later chapter". I am on chapter 5, 6 and 7 here. I feel like a lot of what you guys are talking about are in chapters 20, 21 and so on.

---

I wouldn't think a book introduces do() and while() loops in chapter 20 -- instead they are introduced early, since you can't control flow of a program without them.

I am sure your book introduced these to you by now, the problem is that you don't know how to use control structures properly. The psuedo-code I wrote before is not difficult at all -- that is not chapter 20 stuff to change that to real code. Since you're on chapter 7, you should be able to easily take that pseudo-code and change it to C++, if not, you didn't thoroughly understand chapters 1 through 6, and you need to re-read them again.

Quote:

---

Right now, all my instructor requires is that the program works. It doesn't need to purr like a kitten, it just needs to produce the answer he is looking for.

---

Then that is no teacher of programming. A programming teacher also looks for structure and how you wrote the program, not if you just get the final results. As I stated earlier, if the teacher is of any worth, you using recursion would/should get points deducted from the assignment.

Regards,

Paul McKenzie

The only idea here is to use user defined functions. Just one or two like I have. The other idea I think is to use a checksum to check the integrity of a file. The program doesn't get graded on being flawless. It gets checked to see that a few basics for the week have been implemented. Like will it check to file for integrity and print the int result of the file.

I have attached the actual assignment for you to check out if you are interested in seeing. it gives the specifics of what the program needs to accomplish

Quote:

---

Originally Posted by CyberShot

I would agree with you on the fundamental misunderstandings of functions. We are using
C++ Programming From problem analysis to program design by D.S Malik. Functions are chapters 6 and 7. I am on chapter 7 right now which is User defined functions 2.

---

Then you need to go back to Chapter 5: Control Structures II (Repetition). That's where this stuff is.

well Paul I think you are right. I have read about do, while, for loops and all that. But one week isn't really enough time to fully comprehend them. Not for me anyway. Remember, I have only been at this for 5 weeks. That will not make me a pro at design. Like I said, I will take a look at your comments and try to change the program

With all due respect, I find it strange that you are unable to understand while loops and do-while loops.

I think probably what it comes down to, is that you are in a panic about all you have to do this week, and you are not giving yourself time to stop and think properly about your assignment. "I don't have time to think about it" you might say, but, if you take a deep breath, try to relax and apply your mind to the subject at hand, I would be very surprised if you cannot understand while loops within 60 seconds.

Once you understand the while loop I would again be very surprised if it takes you more than another 90 seconds (given all the help you've been given on this forum) to understand how to correctly apply it to your program. It should only take you 5 minutes to make the changes to your code. However, even if it takes you 20 minutes (which it really shouldn't), we'd only be asking that you use 22.5 minutes out of your whole week to ammend your a program so that it is correctly designed and implemented. It will get you a better grade, and you will have a better understanding.

The thing is, while loops are exactly as they read:

Code:

---

while( the condition is true)
{
  //do whatever is within these curly braces.
}
//If we're here then the condition is not true anymore

---

For example

Code:

---

bool counted_to_five = false;
int count = 1;

//remember ! means 'not'
while(!counted_to_five) //while not counted to five
{
  //If count is equal to 5
  if(count==5)
  {
    //set counted_to_five to true
    counted_to_five = true;
    //This will mean that !counted_to_five will now resolve to false
    //and the while loop will finish.
  }
  else
  {
    //increment count
    ++count;
  }
}

---

You do not need to understand any more than the above while loop to fix your code. Paul has already given you a suitable solution when he wrote the following pseudo code:

Quote:

---

Originally Posted by Paul McKenzie

Code:

---

start program

set selection to 'a'

while selection is not 'q'
{
    display menu and get selection
    if ( selection is 'a') do a stuff
    if ( selection is 'b') do b stuff
} 

end program

---

---

Quote:

---

Originally Posted by CyberShot

The program doesn't get graded on being flawless.

---

I don't think you are in a position to judge on that. I've had a fair amount of experience of marking University Students work (though mainly for physics rather than programming). When I've done marking I've always judged against a flawless answer (and we were encouraged to do so by the head of department). The point is, if the lecturer or marker doesn't do that, then they have no reference point for how to grade the answers, everything becomes relative or subjective and students often get dealt unfair marks - those who get marked too high don't push themselves to learn as they should, and those who get marked too low often become despondent with their results never really understanding why their mark is so low. The only fair thing to do in subject where there are concrete right and wrong anwers it to mark against a 'flawless' answer.

In programming terms I would expect your program to be logically correct - which it isn't at present. The opinion of most here is that your program will get marked down if you leave it as it is. We're trying to help you, and we're not asking you to spend hours on it, but just a number of dedicated minutes instead.

Thank you guys

Okay Paul, I re worked the program. Is this what you meant by doing it correctly?

Code:

---


// IntegrityCheck.cpp : main project file.

#include "stdafx.h"
#include <string>
#include <iostream>
#include <iomanip>
#include <cmath>
#include <fstream>
using namespace std;

void addChecksum();
void runChecksum();

int main()
{
          char choice;
        bool finished = false;

        while(!finished)
        {
                cout << "Please Select: " << endl;
                cout << "  A) Add checksum to specified file: " << endl;
                cout << "  B) Verify integrity of specified file" << endl;
                cout << "  Q) Quit" << endl;
                cin >> choice;
       
                switch (choice)
                {
                        case 'A':
                                addChecksum();
                                break;
                        case 'a':
                                addChecksum();
                                break;
                        case 'B':
                                runChecksum();
                                break;
                        case 'b':
                                runChecksum();
                                break;
                        case 'Q':
                                finished = true;
                                break;
                        case 'q':
                                finished = true;
                                break;

                        default:
                                cout << "Your choice did not match any of the menu options." << endl;
                }
       
        }
       
        system("pause");
    return 0;
}

void addChecksum()
{
        string inputFileName;
        ifstream inputFile;
        string line;
       
        cout << "Specify the file path: ";
        cin.ignore();
        getline(cin, inputFileName);
        inputFile.open(inputFileName.c_str());
        cout << endl;
       
        inputFile.clear();
       
        while(!inputFile.is_open())
        {
                cout << "The file " << inputFileName << " could not be found or opened!" << endl;
                cout << endl;
                return;
        }
       
        cout << endl;
       
        //This will output the text to the screen. I did this for testing purposes
        //while(inputFile.peek() != EOF)
        //{
        //        getline(inputFile, line);
        //        cout << line << endl;
        //}

}

void runChecksum()
{
        string inputFileName;
        ifstream inputFile;
        string lines;
        int checksum;

        checksum = 0;

        while(!inputFile.is_open())
        {
                cout << "Specify the file path: " << inputFileName;
                getline(cin, inputFileName);

                inputFile.open(inputFileName.c_str());
                //trying to convert the inputFile into a number
                //checksum = static_cast<int>(inputFile);
        }

        inputFile.clear();
        cout << "File checksum = " << checksum << endl;
}

---

I didn't read the code in detail, but yes that looks more like what we're talking about.

One thing though, on switch statements you can stack multiple options like so:

Code:

---

switch(choice)
{
        case 'A':
        case 'a':
                addCheckSum();
                break;
               
        case 'B':
        case 'b':
                runChecksum();
                break;
       
        // etc
}

---

What you have now works fine, it's just easier and cleaner to avoid the redundant code.

Quote:

---

Originally Posted by CyberShot

Code:

---

        //trying to convert the inputFile into a number
        //checksum = static_cast<int>(inputFile);

---

---

Just to add another one:

This is definitely not the way to calculate a checksum. A checksum is supposed to indicate that a file has not changed, so it has to depend on the contents of the file. What you have there at best depends on how your C++ compiler/runtime handles the file stream object and may (or may not) change at will, regardless of whether the file has changed or not.

And in addition I doubt that this static cast would compile at all.

But you may want to think about that later, once you got the rest of your program working correctly.

Quote:

---

Originally Posted by CyberShot

Okay Paul, I re worked the program. Is this what you meant by doing it correctly?

---

Yes, with the other observations made by Speedo.

Regards,

Paul McKenzie