How Can i combine one while loop with a for loop?

Question

I am trying to combine these two loops together per my instructions, but I cant figure out how I should do something like this. since my counter i in the for loop can't go inside while loop condition. My teacher wants the program to do one loop only. Thanks in advance. the code basically runs through the a txt file and is supposed to first assign values to an array and in the for loop I assign it to a pointer.

This part I have problem with:

void fillArr(vector <student> &arr, vector <student *> &ptrels){
    student temp;

    ifstream inFile ("index.txt");
    while (!inFile.eof()) {
        inFile >> temp.fName >> temp.Lname >> temp.gpa >> temp.id >> temp.email;
        arr.push_back(temp);
    }
    for (unsigned i = 0; i < arr.size(); i++ ){
        ptrels.push_back(&arr[i]);
    }

//    combine the two above loops
}

Here is the whole code:

> #include <string>
> #include <cstdio>
> #include <iostream>
> #include <vector>
> #include <fstream>
> #include <sstream> using namespace std;
> 
> struct student {
>     string fName, Lname, email, id;
>     double gpa; } ;
> 
> void input(); void fillArr(vector <student> &arr, vector <student *>
> &ptrels); void printFile(vector <student *> pointarray); void
> sortFile(vector <student> &arr, vector <student *> &pointarray);
> 
> int main() {
>     vector <student> elements;
>     vector <student *> ptrels;
>     
>     ifstream inFile;
>     inFile.open("index.txt");
>     
>     int answer;
>     fillArr(elements, ptrels);
>     cout << "Please select:" << endl
>     << "1 = Select All" << endl
>     << "2 = Order A-Z by Name"<<endl
>     << "3 = To exit"<< endl;
>     cin >> answer ;
>     if (answer == 1){
>         printFile(ptrels);
>         main();
>     }
>     if (answer ==2){
>         sortFile(elements, ptrels);
>         printFile(ptrels);
>         main();
>     }
>     if (answer ==3){
>         inFile.close();
>         exit(0);
>     }
>     else {
>         cout << "Invalid Try Again:"<< endl;
>         main();
>     }
>     return 0; }
> 
> void fillArr(vector <student> &arr, vector <student *> &ptrels){
>     student temp;
> 
>     ifstream inFile ("index.txt");
>     while (!inFile.eof()) {
>         inFile >> temp.fName >> temp.Lname >> temp.gpa >> temp.id >> temp.email;
>         arr.push_back(temp);
>     }
>     for (unsigned i = 0; i < arr.size(); i++ ){
>         ptrels.push_back(&arr[i]);
>     }
> 
> //    combine the two above loops }
> 
> 
> 
> void printFile(vector <student *> pointarray){
>     for (unsigned j = 0; j < pointarray.size(); j++){
>         cout << pointarray[j] -> fName << "    ";
>         cout << pointarray[j] -> Lname<< "    ";
>         cout << pointarray[j] -> gpa << "    ";
>         cout << pointarray[j] -> id << "    ";
>         cout << pointarray[j] -> email << "    ";
>         cout << endl;
>     } }
> 
> //swap the elements by pointer. you are swaping the record not the
> pointers. // only sorting by firstname, sort by all 5 void
> sortFile(vector <student> &arr, vector <student *> &pointarray){
>     for(unsigned i = 0; i < arr.size(); ++i){
>         for(unsigned j = i + 1; j < arr.size(); ++j) {
>             if(arr[i].fName > pointarray[j] -> fName) {
>                 swap(arr[i].fName,pointarray[j] ->fName);
>                 swap(arr[i].Lname,pointarray[j] ->Lname);
>                 swap(arr[i].gpa,pointarray[j] ->gpa);
>                 swap(arr[i].id,pointarray[j] ->id);
>                 swap(arr[i].email,pointarray[j] ->email);
>             }
>         }
>     } }

Also I know I am supposed to ask this in a different question but she also want me to do the sorting which is the last function sortFile to sort by all values not only firstName. Plus somehow she hates the swap function, looking for alternatives. Any hints would be appreciated.

Personally I'd resolve it by removing the 2nd argument and the 2nd loop entirely — UKMonkey, Mar 16 '18 at 16:09
Where did you read to use `while (!inFile.eof())` like that? — Lightness Races in Orbit, Mar 16 '18 at 16:09
Never use `eof` as loop condition. Test for stream failure. And you need to do that for the input operation. — Cheers and hth. - Alf, Mar 16 '18 at 16:09
@SombreroChicken can't do that; arr might reallocate the memory leaving floating pointers — UKMonkey, Mar 16 '18 at 16:09
Do read [Why is iostream::eof inside a loop condition considered wrong?](https://stackoverflow.com/q/5605125/10077). — Fred Larson, Mar 16 '18 at 16:10
What is the point of the second `vector`? All it seems to do is make things more complicated and unsafe — UnholySheep, Mar 16 '18 at 16:11
@UnholySheep: _"What is the point of the second `vector`?"_ Good one ^_^ — Lightness Races in Orbit, Mar 16 '18 at 16:11
@UnholySheep since this is an assignment I suppose to teach him a lesson... although I am not sure if it's the right way to do so.... — bolov, Mar 16 '18 at 16:12
Hmm... I genuinely don't see how you can do that in one loop without ending up with dangling pointers or reserving a ridiculous size for arr ... — bolov, Mar 16 '18 at 16:14
@bolov: Same. You can remove the second loop but then you have to add another loop anyway to pre-examine the input file. So meh. I half expect the teacher hasn't thought this through / doesn't know what they're doing. — Lightness Races in Orbit, Mar 16 '18 at 16:15
kindly have a look I updated the question and added the entire code. — Ali RJ, Mar 16 '18 at 16:46

score 3 · Answer 1 · answered Mar 16 '18 at 16:15

3

A naive solution might collapse the two loops into the following:

void fillArr(vector <student> &arr, vector <student *> &ptrels){
    student temp;

    ifstream inFile ("index.txt");
    while (!inFile.eof()) {
        inFile >> temp.fName >> temp.Lname >> temp.gpa >> temp.id >> temp.email;
        arr.push_back(temp);
        ptrels.push_back(&arr.back());
    }
//    combine the two above loops
}

Assuming that arr.empty() is a precondition of this function (if it isn't, the semantics are broken anyway), this almost does the same thing.

Almost.

Trouble is, each arr.push_back(temp) may result in an expansion of arr, invalidating all of the pointers you've put in ptrels so far. The original solution used a second loop so that you know arr is done and dusted before you start messing around with pointers.

We could still use the one-loop solution, as long as we prevented reallocations by reserveing space in the vector. To do that, you would need to know in advance how many elements you will need. To do that, you will need to consume "index.txt" twice, and then you're back to two loops.

So, basically, no you can't do this.

The real solution is to reject the requirement. Hopefully, that's what your teacher wants you to report. I can't think of any real-world use cases for an "indexing" vector in a simple case like this anyway.

Also, watch your I/O methodology.

answered Mar 16 '18 at 16:15

Lightness Races in Orbit

358,771
68
593
989

I'd like to say that clearly the teacher doesn't want the naive solution... but looking at the questions posted here regularly has made me lose all confidence in teachers. – UKMonkey Mar 16 '18 at 16:18
@UKMonkey: Exactly – Lightness Races in Orbit Mar 16 '18 at 16:18
just a ridiculous thought: you could get the size of the file (with a library or system specific API) and reserve the vector for the worst case scenario which is `size_of_file / 5` – bolov Mar 16 '18 at 16:19
@bolov: You _could_. – Lightness Races in Orbit Mar 16 '18 at 16:19
I tried now I am getting addresses of value and not the values themselves. I added the entire code for you to see. – Ali RJ Mar 16 '18 at 16:46
@AliRJ: What does that have to do with _this_ question? Looks like a completely different question? – Lightness Races in Orbit Mar 16 '18 at 17:19
@LightnessRacesinOrbit you mean people don't ask the question they mean to ask? ;) – UKMonkey Mar 16 '18 at 17:20

score 1 · Answer 2 · answered Mar 16 '18 at 17:20

So with your posting of the whole code; the solution is absolutely to remove the need for the vector of pointers entirely.

You make life much harder in your sort function by trying to deal with 2 vectors the way you do; and your printFile() is no simpler working with pointers than it would be with a reference to the vector.

I can't see why you have this vector of pointers at all in the first place; other than sometimes they're used to try and perform sorting on a group withuot changing the order of the original group. Since you do change the order, it makes is a vector of pain.

Get rid of it; make life simple.

Aside: Your sortFile() can use std::sort and become not only faster but easier to read.

score 0 · Answer 3 · answered Mar 17 '18 at 07:07

C++17 code (I left the sort function body empty so as to leave something to do for the homework, and I think that part was meant to be the Main Thing™):

// Source encoding: utf-8 with BOM ∩
#include <string>               // std::(string)
#include <iostream>             // std::(cout, cerr)
#include <optional>             // std::(optional)
#include <utility>              // std::(move)
#include <vector>               // std::(vector)
#include <fstream>              // std::(ifstream)
#include <sstream>              // std::(istringstream)
#include <stdexcept>            // std::(exception, runtime_error)
using namespace std;

auto hopefully( bool const e ) -> bool { return e; }
[[noreturn]] auto fail( string const& s ) -> bool { throw runtime_error( s ); }

auto optional_line_from( istream& stream )
    -> optional<string>
{
    string result;
    return getline( stream, result )? result : optional<string>{};
}

struct Student_info
{
    string first_name;
    string last_name;
    double gpa;
    string id;
    string email;
};

// Originally
// void fillArr(vector <Student_info> &arr, vector <Student_info *> &ptrels):

auto data_from( string const& filename )
    -> vector<Student_info>
{
    ifstream in_file{ "index.txt" };
    hopefully( not in_file.fail() )
        or fail( "Failed to open “index.txt” for reading." );

    vector<Student_info> result;
    while( optional<string> line = optional_line_from( in_file ) )
    {
        Student_info temp;
        istringstream items{ *line };
        items >> temp.first_name >> temp.last_name >> temp.gpa >> temp.id >> temp.email;
        hopefully( not items.fail() )
            or fail( "Failed to parse input line: “" + *line + "”." );
        result.push_back( move( temp) );
    }
    return result;
}

auto operator<<( ostream& stream, Student_info const& item )
    -> ostream&
{
    stream
        << item.first_name << "    "
        << item.last_name<< "    "
        << item.gpa << "    "
        << item.id << "    "
        << item.email;
    return stream;
}

// Originally
// void printFile(vector <student *> pointarray):

void print( vector<Student_info const*> const& pointers )
{
    for( auto const p : pointers ) { cout << *p << endl; }
}

// Originally
// void sortFile(vector <student> &arr, vector <student *> &pointarray):

void sort( vector<Student_info const*>& pointarray )
{
    // TODO:
    // Using std::sort is a good idea, if you're allowed to do that.
    // std::tuple instances can be lexicographically compared with `<`, and
    // `std::tie` produces a tuple. That's great for the current job.
}

// Originally
// int main():

void cpp_main()
{
    vector<Student_info> const data = data_from( "index.txt" );

    vector<Student_info const*> pointers;
    for( Student_info const& item : data ) { pointers.push_back( &item ); }

    for( ;; )
    {
        cout << "Please select\n"
             << "1 = Select All\n"
             << "2 = Order A-Z by Name\n"
             << "3 = To exit"
             << endl;
        try
        {
            switch( stoi( *optional_line_from( cin ) ) )
            {
                case 1:
                {
                    print( pointers );
                    break;
                }
                case 2:
                {
                    sort( pointers );
                    print( pointers );
                    break;
                }
                case 3:
                {
                    return;
                }
                default:
                {
                    fail( "" );
                }
            }
        }
        catch( ... )
        {
            // Avoid infinite loop on input failure:
            if( cin.fail() )
            {
                fail( "Failed reading standard input." );
            }
            cout << "Invalid Try Again:" << endl;
        }
    }
}

auto main()
    -> int
{
    try
    {
        cpp_main();
        return EXIT_SUCCESS;
    }
    catch( exception const& x ) 
    {
        cerr << "!" << x.what() << endl;
    }
    return EXIT_FAILURE;
}

How Can i combine one while loop with a for loop?

3 Answers3