theyCallMeJun
theyCallMeJun

Reputation: 941

How do i abstract the common parts of Java for loop so as to avoid repetition/duplication

I have a method that receives a list of things, iterates the list and does XYZ things on every iteration. The definition of "XYZ things" depends on the implementation or what I'm trying to achieve. For some i just want to run some SQL that hits the database and updates customer name and invoice due date. For others i just want to update tax amount, days past due etc. The situation is always different. How do i create just one reusable method rather than duplicate this method all over which isn't so much in-the-spirit of "OnceAndOnlyOnce".

  DaoProvider daoProvider;

  public void buildAndUpdateCustomer(List<Object> list) {
    for (Object obj : list) {
      Map<String, Object> m = (Map<String, Object>) obj;
      Map<String, Object> args = new HashMap<>();
      for (Map.Entry<String, Object> x : m.entrySet()) {
        args.put(x.getKey(), x.getValue());
      }
      //these are my XYZ things...
      daoProvider.updateCustomerName();
      daoProvider.updateAgingMia();
    }
}


  public void buildAndUpdateTax(List<Object> list) {
    for (Object obj : list) {
      Map<String, Object> m = (Map<String, Object>) obj;
      Map<String, Object> args = new HashMap<>();
      for (Map.Entry<String, Object> x : m.entrySet()) {
        args.put(x.getKey(), x.getValue());
      }
      daoProvider.updateTaxAmount();
    }
}

  public void buildAndUpdateLedgerBal(List<Object> list) {
    for (Object obj : list) {
      Map<String, Object> m = (Map<String, Object>) obj;
      Map<String, Object> args = new HashMap<>();
      for (Map.Entry<String, Object> x : m.entrySet()) {
        args.put(x.getKey(), x.getValue());
      }
      daoProvider.updateLedgerBalance();
    }
}

Upvotes: 0

Views: 116

Answers (2)

Boris the Spider
Boris the Spider

Reputation: 61198

I'm going to assume that args needs to be passed into the methods thusly:

public void buildAndUpdateCustomer(List<Object> list) {
    for (Object obj : list) {
      Map<String, Object> m = (Map<String, Object>) obj;
      Map<String, Object> args = new HashMap<>();
      for (Map.Entry<String, Object> x : m.entrySet()) {
        args.put(x.getKey(), x.getValue());
      }
      //these are my XYZ things...
      daoProvider.updateCustomerName(args);
      daoProvider.updateAgingMia(args);
    }
}

Then we can write a method:

public void buildAndProcess(List<Object> list, Consumer<Object> processor) {
    for (Object obj : list) {
      Map<String, Object> m = (Map<String, Object>) obj;
      Map<String, Object> args = new HashMap<>();
      for (Map.Entry<String, Object> x : m.entrySet()) {
        args.put(x.getKey(), x.getValue());
      }
      processor.accept(args);
    }
}

And your previous method becomes:

public void buildAndUpdateCustomer(List<Object> list) {
    buildAndProcess(list, args -> {
      //these are my XYZ things...
      daoProvider.updateCustomerName(args);
      daoProvider.updateAgingMia(args);
    });
}

This chunk of code is odd:

Map<String, Object> m = (Map<String, Object>) obj;
Map<String, Object> args = new HashMap<>();
for (Map.Entry<String, Object> x : m.entrySet()) {
    args.put(x.getKey(), x.getValue());
}

It identical to Map<String, Object> args = new HashMap<>((Map<String, Object>) obj);


I will reiterate what I said in the comments:

Just loooking at your code, you have far too maybe Object - Java is a strongly typed language and when someone has so many Object and so much casting this is a screaming red flag. Generally, one shouldn't see much usage of Object at all

You seem to be using List<Object> and Map<String, Object> as essentially generic arguments - this is a really nasty antipattern, it removes all of the type safety the compiler provides and even removes the name checking.

Upvotes: 0

Andreas
Andreas

Reputation: 159260

You can move the common code to a private helper method:

@SuppressWarnings("unchecked")
private static Map<String, Object> getArgs(Object obj) {
    Map<String, Object> m = (Map<String, Object>) obj;
    Map<String, Object> args = new HashMap<>();
    for (Map.Entry<String, Object> x : m.entrySet()) {
        args.put(x.getKey(), x.getValue());
    }
    return args;
}

public void buildAndUpdateCustomer(List<Object> list) {
    for (Object obj : list) {
        Map<String, Object> args = getArgs(obj);
        // these are my XYZ things...
        daoProvider.updateCustomerName(args);
        daoProvider.updateAgingMia(args);
    }
}

public void buildAndUpdateTax(List<Object> list) {
    for (Object obj : list) {
        Map<String, Object> args = getArgs(obj);
        daoProvider.updateTaxAmount(args);
    }
}

public void buildAndUpdateLedgerBal(List<Object> list) {
    for (Object obj : list) {
        Map<String, Object> args = getArgs(obj);
        daoProvider.updateLedgerBalance(args);
    }
}

Or with Java 8 you can do it with lambdas:

@SuppressWarnings("unchecked")
private static void buildAndUpdate(List<Object> list, Consumer<Map<String, Object>> consumer) {
    for (Object obj : list) {
        Map<String, Object> m = (Map<String, Object>) obj;
        Map<String, Object> args = new HashMap<>();
        for (Map.Entry<String, Object> x : m.entrySet()) {
            args.put(x.getKey(), x.getValue());
        }
        consumer.accept(args);
    }
}

public void buildAndUpdateCustomer(List<Object> list) {
    buildAndUpdate(list, args -> {
        daoProvider.updateCustomerName(args);
        daoProvider.updateAgingMia(args);
    });
}

public void buildAndUpdateTax(List<Object> list) {
    buildAndUpdate(list, args -> daoProvider.updateTaxAmount(args));
}

public void buildAndUpdateLedgerBal(List<Object> list) {
    buildAndUpdate(list, args -> daoProvider.updateLedgerBalance(args));
}

Upvotes: 1

Related Questions