Logic is incorrect in the flow of loops and ifs

Question

So I'm trying to find the lowest year in pat.txt and highest year in don.txt:

Pat.txt

Androf O kidney 24 2012
Blaren B kidney 35 2010
Cosmer A kidney 35 2000
Eralod O heart 53 2009
Forend B kidney 31 2003

Don.txt

Zerk B kidney 20 2009
Rampe A kidney 31 2005
Darech B kidney 34 2008
Seo A kidney 26 2010
Yuio B kidney 26 2013

Code is as follows:

 struct Person {

    string surname;
    string BType;
    string organ;
    int age;
    int year, ID, IDp;
} Patient[50], Donor[50];

Then code of interest:

int Date = 5000;
        int Datel = 1000;
            for (i = 0; i < 6; i ++){
                    for (i1 = 0; i1 < 6; i1++){

                            if ((Patient[i].BType == Donor[i1].BType) && (Patient[i].organ == Donor[i1].organ)){

                                    if (Patient[i].year < Date){
                                        Date = Patient[i].year;

                                    //}
                                        if ((Patient[i].year == Date ) && (Donor[i1].year > Datel)){
                                            Date = Patient[i].year;
                                            Datel = Donor[i1].year;
                                            cout << Date << "   " << Datel << "\n";

                                        }

                                    }
                            }
                        }
                }

Currently I get the highest patient and lowest donor (2010-2009), I need to obtain (2003-2013)

I feel like the logic is incorrect in one of the if statements that is doing the equality. At the moment it is finding the highest patient and lowest doner. I have to flip them. Spent hours trying to figure this out, i would really appreciate it if someone can see my mistake. I'm sure its a silly little mistake, but for the life of me I just cant find it

Please use better variables names.. it makes people trying to read your code able to follow it much easier. What's "Date" and "Date1"? Is one supposed to be the year of the oldest patient? Why not call them "newestPatientDate" and "oldestDonerDate"? — PherricOxide, Oct 10 '13 at 23:08
Go to the code which you feel least confident about, ask a question about it, then try to answer that question. — Leonardo, Oct 10 '13 at 23:12

john · Answer 1 · 2013-10-10T23:14:44.477

I'm confused, why not the simple code?

int oldestPatientDate = 5000;
int newestDonorDate = 1000;
for (i = 0; i < 6; i++)
    if (oldestPatientDate > Patient[i].year)
        oldestPatientDate = Patient[i].year;
for (i = 0; i < 6; i++)
    if (newestDonorDate < Donor[i].year)
        newestDonorDate = Donor[i].year;
cout << oldestPatientDate << "   " << newestDonorDate << "\n";

But maybe there's something more to this task than you've described.

Completely agree with PherricOxide about the better variable names. Programming is easier if you choose good variable names.

Logic is incorrect in the flow of loops and ifs

1 Answers1