1

I'm using a foreach loop to go through an array list of objects that are all created in different subclasses of the same superclass, then if statements with instanceof boolean expressions to check which subclass the particular item the each loop is now on belongs to, but I don't think I have the boolean expression using instance of correct, because while debugging my code all the if statements just get skipped over.

for (Appointment item: AppointmentBook.apps){


         if (item instanceof Onetime){
             boolean checkOnce = ((Onetime) item).occursOn(month, day, year);

             if (checkOnce == true){
                 appointmentsToday++;
                 appsToday.add(item);

             }//check once true 

             else appointmentsToday = 0;

         }//if onetime

Appointment is the superclass of Onetime. AppointmentBook the class where the array list of appointments is located. occursOn is a method in the Onetime class

  • full code + initialization of AppointmentBook.apps would be great –  Mar 15 '15 at 15:40
  • Be sure if in your collection there are objects that extends/implements class/interface Onetime. If there are such object's i'am sure that result of operator instanceof will be true. – Piotr Zych Mar 15 '15 at 15:42
  • Shouldn't `occursOn` be on `Appointment`? That way, you can simply do `if(item.occursOn(month, day, year) { ... }`. Will also save you the expensive `instanceof` check. – manish Mar 15 '15 at 15:42
  • @manish i think that it is not question which model of programming he should use, but about stricte operator instanceof. – Piotr Zych Mar 15 '15 at 15:44

2 Answers2

0

You should always avoid code like this:

if (item instanceof TypeA) {
    ...
} else
if (item instanceof TypeB) {
    ...
} else ...

Use polymorphism or you'll suffer from high coupling.

Everv0id
  • 1,862
  • 3
  • 25
  • 47
0

Your boolean expression using ìnstanceof`is correct. I would suspect that the method that you use to fill the apps static field from AppointmentBook class is the source of the issue. That's the only logical explanation if debugging shows that every if statement is being skipped. I tried to reproduce some code similar to yours in order to test it and it is working fine.

Here is what I did

First an Appointment class:

public class Appointment {

}

Second an AppointmentBook class

import java.util.ArrayList;
import java.util.List;

public class AppointmentBook {

    public static List<Appointment> apps = new ArrayList<Appointment>();

    public AppointmentBook addAppointment(Appointment app) {
        apps.add(app);
        return this;
    }

}

Third a OneTime class that extends Appointment (since you said Appointment is the superclass of OneTime)

public class OneTime extends Appointment {

    public boolean occursOn(int month, int day, int year)  {
        if (day >= 15) {
            return true;
        } else {
            return false;
        }
    }
}

As you can see I am using a trivial test case to return boolean results from the occursOn method (simply for test purpose)

And then I created the following test class. I fill the AppointmentBook apps with four Appointment instances, of which two are "instanceof" OneTime

public class AppointmentTest {

    static int year = 2015;
    static int month = 3;
    static int day = 15;

    public static void main(String[] args) {

        AppointmentBook book = new AppointmentBook();
        book.addAppointment(new Appointment())
        .addAppointment(new OneTime())
        .addAppointment(new Appointment())
        .addAppointment(new OneTime());

        for (Appointment item: AppointmentBook.apps) {

            if (item instanceof OneTime) {
                boolean checkOnce = ((OneTime)item).occursOn(month, day,    year);

                if (checkOnce == true) {
                    System.out.println("We have a checked OneTime     instance...");
                } else {
                    System.out.println("We have an unchecked OneTime     instance...");
                }
            } else {
                System.out.println("Not a OneTime instance...");
            }           
        }       
    }
}

The result obtained is shown in the following image: it proves that your instanceof expression is correct and the issue most probably is related to the method that fills the apps field

enter image description here

alainlompo
  • 4,414
  • 4
  • 32
  • 41