0

I have just started with c++ and this is for a project that I'm working on. The problem is that I am not able to return a string from the SearchFunction to main. The search function itself is working perfectly and easily finding and displaying the row its supposed to find but the string temp remains empty despite me returning a string from Search Function. As a result, the DeleteFunction is not working because its not being passed the string that it's supposed to delete.

I have tried using pointers instead of returning value but still the result is same. Please help me understand where I'm going wrong.

#include<iostream>
#include<stdio.h>
#include<string>
#include<fstream>
using namespace std;

string data,temp;

string SearchFunction(string);                  
void DeleteFunction(string);                    

int main()
{
    int choice=0,choice3=0;
    char yn1;
    string search;

    cout<<"1. Press 1 to delete."<<endl;
    cin>>choice;
    cin.clear();                                        
    cin.ignore(1000,'\n');                              

    if(choice==1)
    {   
        cout<<"Enter RegNo. of record to be deleted: ";
        getline(cin,search);                        
        search="RegNo.: "+ search;                  //Concatenate with "RegNo: " to ensure that the search is done "by RegNo".
        temp=SearchFunction(search);                

        cout<<"1. "<<temp<<"\n\n";
        cout<<temp.length()<<endl;

        cout<<"Are you sure you want to delete the above record of"<<search<<"? Y/N";
        yn1=getchar();
        cin.clear();                                        
        cin.ignore(1000,'\n');                          

        if(!(yn1=='y' || yn1=='Y' || yn1=='n' || yn1=='N'))
        {
            do
            {
                cout<<"Enter 'Y' or 'N': ";
                yn1=getchar();
            }while(!(yn1=='y' || yn1=='Y' || yn1=='n' || yn1=='N'));
        }

        if(yn1=='y' || yn1=='Y')
        {
            DeleteFunction(temp);                   //Call delete function to delete record.
        }
    }
    return 0;
}


string SearchFunction(string search)
{

    int found=0, check=0;                               //Declare and initialize both variables to 0.
    ifstream outfile;                                   //Create object for reading file.

    outfile.open("student.txt");                        //Open file.

    while(!outfile.eof())                               //Continue loop until the end of file.
    {
        found=0, check=0;                               //Initialize both variables to 0 again in anticipation of repititions.

        getline(outfile, data);                         //Input one row from file to string variable data.
        found=data.find(search, found);                 //Search for the search term in string data.
        if(found!=string::npos)                         //If search term found.
        {
            cout<<data<<endl;                           //Display row.
        }
    }
    outfile.close();

    return data;
}

void DeleteFunction(string temp)
{
    string line;
    ifstream in("student.txt");
    if( !in.is_open())
    {
        cout << "Input file failed to open\n";
    }

    ofstream out("temp.txt");

    while( getline(in,line) )
    {
        if(line != temp )
            out << line << "\n";
    }
    in.close();
    out.close();    

    remove("student.txt");
    rename("temp.txt","student.txt");
}
8
  • 1
    First, while (!eof()) is wrong. Also, there's no reason for that returned string to be a global variable. Commented Dec 18, 2013 at 19:47
  • Note data is a global variable why do you have to return it ? you can just access it in main after calling the function. Commented Dec 18, 2013 at 19:47
  • 2
    Why ? when using comparison operators on string everything is done underneath using compare function. Commented Dec 18, 2013 at 19:51
  • 1
    @ThomasMatthews: They do different things. One results in a boolean; the other in a tri-state comparison result. Commented Dec 18, 2013 at 19:58
  • 1
    @user3116554, Well, as soon as you call the function again, it changes. If another function didn't expect that, kaboom. It's worse with multithreading. Commented Dec 18, 2013 at 20:43

2 Answers 2

1

You have stop reading the file after you found the row you are looking for. Maybe you want to change the function to:

string SearchFunction(string search)
{

    int found=0, check=0;                               //Declare and initialize both variables to 0.
    ifstream outfile;                                   //Create object for reading file.

    outfile.open("student.txt");                        //Open file.

    // Also check if found!!!
    while(!outfile.eof() && !found)                     //Continue loop until the end of file.
    {
        found=0, check=0;                               //Initialize both variables to 0 again in anticipation of repititions.

        getline(outfile, data);                         //Input one row from file to string variable data.
        found=data.find(search, found);                 //Search for the search term in string data.
        if(found!=string::npos)                         //If search term found.
        {
            cout<<data<<endl;                           //Display row.
        }
    }
    outfile.close();

    return data;
}
Sign up to request clarification or add additional context in comments.

10 Comments

if you found it why to keep running in while ? why just not to return after cout inside the while ?
No no, that's what you are doing. And it's wrong. Sorry I meant: "You must not keep running in while" That's why you have to check in while statement if found become true. See the code I posted.
Not me :) , and yes you are right I confused the statement of While, still Imo it is better to return inside the if inside the while , and then outside the while you can return string if you didnt find anything , thus you can handle not found at all .
@user2957713 I'm agree with you, but since he said he has just started with c++, I wanted give him a response with the less change to the code as possible. Move the return statement, or even add another might not be so easy to understand for him.
@user3116554 also please use data and temp as local variables. Globals are bad. It won't change anything in this part of code -> but it is a good initiation of bad habits that may result in future unwanted behavior
|
1

You need to break out of the while loop when you've found your data. A simple way is to just return at that point.

Don't use globals unless you have some very good reason. Globals used as scratch-pad variables, as above, are just Evil™.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.