8

I have an object which has a few arrays as fields. It's class roughly looks like this:

public class Helper {
    InsuranceInvoices[] insuranceInvoices;
    InsuranceCollectiveInvoices[] insuranceCollectiveInvoices
    BankInvoices[] bankInvoices;
    BankCollectiveInvoices[] bankCollectiveInvoices;
}

All of the invoice types have a mutual marker interface Invoices.
I need to get all of the invoices to invoke another method on them.

Helper helperObject = new Helper();
// ...

for (InsuranceInvoices invoice : helperObject.getInsuranceInvoices()) {
    Integer customerId = invoice.getCustomerId();
    // ...
}
for (BankInvoices invoice : helperObject.getBankInvoices()) {
    Integer customerId = invoice.getCustomerId();
    // ... 
}

// repeat with all array fields

The problem is that all invoices only have the marker interface in common. The method getCustomerID() is not defined by a mutual interface or class. This is a behaviour I cannot change due to a given specification.

The code repetition inside the for-each-loop is something that bugs me. I have to do the exact same thing on all invoice objects in the four different arrays. Hence four for-each-loops that unecessary bloat the code.

Is there a way that I can write a general (private) method? One idea was:

private void generalMethod(Invoice[] invoiceArray){
    // ...
}

But this would require four instanceof checks because the class Invoice doesn't know the method getCusomterId(). Therefore I would gain nothing; the method would still contain repetitions.

I'm thankful for every possible solution to generalize this problem!

DeMo
  • 683
  • 5
  • 14
  • 1
    so, the classes all implement `Invoice`, and all implement `getCustomerId`, but `getCustomerId` is not in the `Invoice` interface? You'll have to use reflection to access the method by name and invoke it. At this point, the `Invoice` interface is quite useless. – njzk2 Jun 18 '15 at 13:21
  • Do `getInsuranceInvoices`, `getBankInvoices`...all four return same number of invoices? – Rajesh Jun 18 '15 at 13:31
  • 3
    I have a suggestion... apply a cluebat to the person who made the `Invoice` Interface a marker interface instead of one that specifies what methods invoices have in common. – Powerlord Jun 18 '15 at 14:14
  • I don't have enough to provide a full answer, but sounds like you're looking for duck typing: https://en.wikipedia.org/wiki/Duck_typing The wiki shows using dynamic, and there are c# duck typing solutions out there, but it may not be worth the effort – David Ferretti Jun 18 '15 at 14:51
  • @njzk2 That's right. Reflections might be a way to go but it is often considered as kind of "dirty", isn't it? – DeMo Jun 19 '15 at 06:03
  • @Powerlord Yes, it's a poor design. However, the example I gave is very simplified. Nonetheless the problematic is still the same. – DeMo Jun 19 '15 at 06:03
  • @David This would definitely work, but the resulting overhead for this rather simple problem might definitely not be worth the effort :). Thank you anyway! – DeMo Jun 19 '15 at 06:03

3 Answers3

7

Possible solutions to generalize the problem (ordered from best to worst):

Using wrapper class

public class InvoiceWrapper {
    private String customerID;
    public String getCustomerID() {
        return customerID;
    }
    public InvoiceWrapper(BankInvoices invoice) {
       this.customerID = invoice.getCustomerID();
    }
    public InvoiceWrapper(InsuranceInvoices invoice) {
       this.customerID = invoice.getCustomerID();
    }
    // other constructors
}

Upd If I understood correctly, you need to do something with IDs in all arrays. To use InvoiceWrapper, you also need to implement iterator in Helper class, that will walk through arrays and return a wrapper for each entry. So, you will have code that works with 4 arrays anyway.

Using instance of casts

public class CustomerIdHelper {
    public static String getID(Invoice invoice) {
        if (invoice instanceof InsuranceInvoices) {
            return ((InsuranceInvoices) invoices).getCustomerID();
        } else if ...
    }
}

Calling methods by name via Reflection

public class CustomerIdHelper {
    public static String getID(Invoice invoice) {
        Method method = invoice.getClass().getDeclaredMethod("getCustomerId");
        return (String) method.invoke(invoice);
    }
}
AdamSkywalker
  • 11,408
  • 3
  • 38
  • 76
  • Thank you for your suggestions! The first approach is something I did not take into consideration. But could you explain to me why you chose the Reflection approach to be the worst of the three? – DeMo Jun 19 '15 at 06:11
  • 1
    @DeMo That's because if someone removes method, you will not notice it. In first two variants you will see compilation error. – AdamSkywalker Jun 19 '15 at 09:22
2

It's not pretty, but you could use reflection to look up the getCustomerId Method and then invoke() it, cf. Class.getDeclaredMethod().

private void generalMethod(Invoice[] invoiceArray){
  try {
    for (Invoice invoice : invoiceArray) {
      Method getCustomerId = invoice.getClass().getDeclaredMethod("getCustomerId");
      getCustomerId.invoke(invoice);
    }
  } catch (Exception e) {
    // ...
  }
}

Do note that this is untested.

jensgram
  • 31,109
  • 6
  • 81
  • 98
2

If you are not allowed to change the classes you are handling by adding a custom interface to them. The best thing you can do is wrap them with a custom class that does have the desired properties.

This way you will have one class with all 'not so nice' code that converts the classes you can not touch to nice classes that match a proper and useful design.

For instance you could have a class WrappedInsuranceInvoice that extends WrappedInsurace and contains a member field InsuranceInvoice. If you don't need to keep the original class you would be off even better by copying the data. This way you could for instance lose the arrays and use lists instead.

Thirler
  • 20,239
  • 14
  • 63
  • 92