I am running into this issue where I want the largest and smallest value of id from a file that I am reading. This file contains other information as well. I am successfully able to identify the largest value but the smallest is just being set to the last value that gets read, which is not the smallest in the file. Here is the code

largestId = 0;
smallestId = 99999;
while(theFile >> firstName >> lastName >> id)
{
   if(id > largestId){
            largestId = id;
   }else if(id < smallestId){
            smallestId = id;
   }
}

CodePudding user response：

You don't check for error states. What about the situation were there are no values in the file. Or only one value in the file.

You make assumptions about the size of largest and smallest values by using magic numbers.

if (theFile >> firstName >> lastName >> id)
{
    // You have at least one value it is the largest and smallest value.
    smallestId = largestId = id;


    while(theFile >> firstName >> lastName >> id)
    {
       // There is an argument that this would be better to write
       // as a function call as that would be self documenting.
       //  id = std::max(id, largestId);
       if(id > largestId){
                largestId = id;
       }
       if(id < smallestId){
                smallestId = id;
       }
    }
}
else {
    // ERROR
}

CodePudding user response：

Here is more fancy way you could do it by using algorithms and iterators:

struct Person 
{
    std::string firstName;
    std::string lastName;
    int id;
};

std::istream& operator>>(std::istream& in, Person& person)
{
    return in >> person.firstName >> person.lastName >> person.id;
}

bool find_min_max(std::istream& in, int& min, int& max)
{
    using Iter = std::istream_iterator<Person>;
    auto a = std::minmax_element(Iter{in}, {},
        [](const auto& a, const auto& b) { return a.id < b.id; });
        
    if (a.first != Iter{}) 
    {
        min = a.first->id;
        max = a.second->id;
    }

    return a.first != Iter{};
}

https://godbolt.org/z/dY8KqzzxM