EOF rearranges my priority queue?

[Miles Hikari]

At the moment I'm doing some debugging of a school project I wasn't able to finish last semester, determined to figure it out in my free time. I've got some code that simply read in several lines from data file containing a category char, a name/code string a priority int between 1 and 10, and a description string. The goal is to make a custom priority Queue set up so that the items will be automatically be sorted with the most important matters first (M>C>U, 10>0)

With a little help from someone earlier I was able to get the priority queue working perfectly. It displayed everything I needed it too in the correct order. Except for some reason I found that it was displaying whatever was at the bottom of my data file twice, after some debugging I found that the isn't oddly wasn't the display function, but rather that particular bit of data was getting input twice. So I switched my input code from "while (data)" to "while (!data.eof)" and it fixed the problem of my code doubling up on the bottom of my list.... But now my priority queue for some reason is all jumbled, with Cs about and below Ms, numbers having no priority.... I'm really confused why because all I changed was how the program was checking if I was out of data or not.... Can someone help here? It's the last thing I need to put this challenge behind me. (also please ignore the obscene number of includes in my header file, I had copied most of them over from another program of mine and just was too low on time at the time to delete the ones i didn't need.

alert.h

 #ifndef ALERT_H
#define ALERT_H
#include <iostream>
#include <cmath>
#include <random>
#include <ctime>
#include <string>
#include <fstream>
#include <stdio.h>      /* printf, scanf, puts, NULL */
#include <stdlib.h>     /* srand, rand */
#include <time.h>       /* time */

using namespace std;

class alert
{
public:
    alert();
    alert(char c, string i, int p, string d)
    {
        cat = c;
        id = i;
        pri = p;
        desc = d;

        if (c == 'M')
            catPri = 3;
        else if (c == 'C')
            catPri = 2;
        else catPri = 1;
    }



    void display() const
    {
        cout << cat << " " << id << " " << pri << " " << desc << endl;
    }

    int getCatPri()
    {
        return catPri;
    }

    int getPri()
    {
        return pri;
    }
    char getCat()
    {
        return cat;
    }
    string getID()
    {
        return id;
    }
    string getDe()
    {
        return desc;
    }
private:
    char cat;
    string id;
    int catPri;
    int pri;
    string desc;
};


class Compare
{
public:
    bool operator() (alert a, alert b) const
    {
        if (a.getCatPri() >= b.getCatPri() && (a.getPri() >= b.getPri()))
            return false;
            else return true;
    }
};

#endif 

#include "alert.h"
#include <queue>



int main (int argc, char** argv)
{
    if (argc != 2)
    {
        cerr << "Usage: " << argv[0] << "inFileName" << endl;
        return -1;
    }

    alert * aptr;
    char cat;
    string id;
    int pri;
    string desc;
    ifstream data (argv [1]);
    priority_queue<alert, vector<alert>, Compare> qu;
    while (!data.eof()) /// Was originally "while(data)", but caused Scandal to get input twice.
    {

        data >> cat >> ws;
        data >> id >> ws;
        data >> pri >> ws;
        getline(data, desc);

        aptr = new alert(cat,id, pri, desc);
        //*aptr->display();
        qu.push(*aptr);
    }

cout << qu.size() << endl;
    while (!qu.empty())
    {


        qu.top().display();
        qu.pop();

    }

    return 0;
}

and here's the data file I'm inputting

C L1200 6 Lunch Break

M D2012 10 World is Ending

U P345 1 Leaky Diaper

M T972 3 Missing Cat

U A847 10 Amber Alert

C L524 7 Bankrupt

U A360N 10 Anime Marathon

M D552 5 Low Ammo

U K3953 9 Blue Jester Needs Food Badly

C BH90210 10 Scandal

[jlb]

. But now my priority queue for some reason is all jumbled,

Well there is little difference between while(data) and while(!data.eof()), the fist checks for all errors the second only checks for eof(). So the "new" problem is that there is probably some bad data in your file.

However neither are really the best to use for a data entry loop. You really should use the actual read to control the loop.

    while (data >> cat >> ws >> id >> ws >> pri >> ws && getline(data, desc)) 
    {

        

What is with all the ws calls? Do you realize that the extraction operator>> skips leading whitespace for most of the types?

Is there a leading space on your data? And is there truly an extra line between every line?

In future please place your file input information inside code tags to preserve formatting?

What operating system are you using?

[mbozzi]

Don't loop on eof(). That's almost always wrong. You can find plenty written about it.

Instead, perform the read operation in the loop condition. This is idiomatic -- see below: I refactored your code and fixed the bugs I could find.

#include <iostream>
#include <fstream>
#include <queue>
#include <tuple>

class alert {
public:
    alert() = default; // don't leave functions undefined;
    alert(char c, std::string i, int p, std::string d)
        : cat(c), id(i) // use initializer lists
        , pri(p), desc(d) {} // no definition needed

    // compute the result instead of caching it -- otherwise
    // if cat is changed, catPri would not update.
    int getCatPri() const { 
        if (cat == 'M') return 3;
        if (cat == 'C') return 2;
        else return 1;
    }
    
    int getPri()        const { return pri;  }
    char getCat()       const { return cat;  }
    std::string getID() const { return id;   }
    std::string getDe() const { return desc; }

    friend std::istream& operator>>(std::istream& s, alert& a) {
        s >> a.cat >> std::ws >> a.id >> std::ws >> a.pri >> std::ws;
        return getline(s, a.desc);
    }
    
private:
    char cat;
    std::string id;
    int pri;
    std::string desc;
};

// replaces alert::display()
// it can be implemented in terms of the public interface -- 
// it doesn't need direct access to the members of alert. 
std::ostream& operator<<(std::ostream& s, alert const& a) {
    return s << a.getCat() << ' ' << a.getID() << ' '  
             << a.getPri() << ' ' << a.getDe();
}

// replaces the compare class
// the result of forward_as_tuple is compared in lexicographic order.
// This is a tidy way to compare things correctly.
bool operator<(alert const& a, alert const& b) {
    return std::forward_as_tuple(a.getCatPri(), a.getPri()) <
           std::forward_as_tuple(b.getCatPri(), b.getPri());   
}

int main (int argc, char** argv){
    if (argc != 2) {
        std::cerr << "Usage: " << argv[0] << "inFileName\n";
        return -1;
    }

    std::ifstream data{argv[1]};
    std::priority_queue<alert> qu;
    for (alert a; data >> a; ) qu.push(a); 

    std::cout << qu.size() << '\n';
    for(; qu.size(); qu.pop()) std::cout << qu.top() << '\n';
}

Demo:
http://coliru.stacked-crooked.com/a/3c77dbc74ed1c75e