How can I factorise this code? [duplicate]

Question

I'm working on a java webserver which calls a web service. I'm trying to write some code in a nicer.

I've got 2 similar functions, only one thing changes : the function i have to call depending what i want to do.

public Object updateDef() { ExecutorService pool = Executors.newFixedThreadPool(20); List<Callable<String>> tasks = new ArrayList<Callable<String>>(); logger.info("Updating surv def..."); for (final Surv surv : Client.activatedSurvs) { final Client client = new Client(surv.getName(), surv.getPassword()); tasks.add(new Callable<String>() { public String call() throws Exception { HERE -> client.updateDef(surv); return surv.getId(); } }); client.destroy(); } List<Future<String>> results = null; try { results = pool.invokeAll(tasks); for (Future<String> future : results) { future.get(); } } catch (ExecutionException e) { // TODO Auto-generated catch block e.printStackTrace(); } catch (InterruptedException e) { // TODO Auto-generated catch block e.printStackTrace(); } pool.shutdown(); return null; }

And the other one :

public Object updateStats() { ExecutorService pool = Executors.newFixedThreadPool(20); List<Callable<String>> tasks = new ArrayList<Callable<String>>(); logger.info("Updating surv Stats..."); for (final Surv surv : Client.activatedSurvs) { final Client client = new Client(surv.getName(), surv.getPassword()); tasks.add(new Callable<String>() { public String call() throws Exception { HERE -> client.updateStats(surv).toString(); return surv.getId(); } }); client.destroy(); } List<Future<String>> results = null; try { results = pool.invokeAll(tasks); for (Future<String> future : results) { future.get(); } } catch (ExecutionException e) { // TODO Auto-generated catch block e.printStackTrace(); } catch (InterruptedException e) { // TODO Auto-generated catch block e.printStackTrace(); } pool.shutdown(); return null; }

So the 2 functions are updateStats and updateDef from class A with a static list from B. The problem is that I've to pass a parameter which is an existing element from this list.

=> A.updateStats(B.list.get(X));

=> A.updateDef(B.list.get(X));

I visited this link How to pass a function as a parameter in Java? but without success

Any idea ?

this question would be better suited on codereview.stackexchange.com — Sionnach733
– Sionnach733, Commented Apr 29, 2014 at 14:24
Pass a boolean in the parameters of the function, something like 'isDef' and use with an if statement to call either one of your update lines — Lex Webb
– Lex Webb, Commented Apr 29, 2014 at 14:25
The question you link gives a perfectly valid answer. Any form of Interface would work, too. — njzk2
– njzk2, Commented Apr 29, 2014 at 14:26
I forgot to say that it has to be the quickest way possible. I'm worrying if statement will slow down my app ? — MacBidouille
– MacBidouille, Commented Apr 29, 2014 at 14:31

njzk2 · Accepted Answer · 2014-04-29 14:40:49Z

Your object is to replace:

tasks.add(new Callable<String>() { public String call() throws Exception { client.updateStats(surv).toString(); return surv.getId(); } });

With

tasks.add(new Callable<String>() { public String call() throws Exception { doSomething(client, surv); return surv.getId(); } });

Which can be done by defining an Interface:

public interface ClientUpdate { public void performUpdate(Client client, Surv surv); }

Then, use this interface as a parameter to your method:

public Object updateStats() {

Becomes

public Object update(ClientUpdate updater) {

Then use it in your callable

tasks.add(new Callable<String>() { public String call() throws Exception { updater.performUpdate(client, surv); return surv.getId(); } });

Then implement it:

updateStats();

becomes:

update(new ClientUpdate() { public void performUpdate(Client client, Surv surv) { client.updateStats(surv).toString(); } });

Similarly for updateDef();

jgitter · Accepted Answer · 2014-04-29 14:24:18Z

You could use a Strategy pattern or Functors. Honestly, I think I would just factor the common pieces out into shared methods.

duffymo · Accepted Answer · 2014-04-29 14:28:42Z

Easy - pass in the Callable and don't repeat all the boilerplate around it.

public Object foo(Callable<String> callable) { ExecutorService pool = Executors.newFixedThreadPool(20); List<Callable<String>> tasks = new ArrayList<Callable<String>>(); logger.info("Updating surv def..."); for (final Surv surv : Client.activatedSurvs) { final Client client = new Client(surv.getName(), surv.getPassword()); tasks.add(callable; client.destroy(); } List<Future<String>> results = null; try { results = pool.invokeAll(tasks); for (Future<String> future : results) { future.get(); } } catch (ExecutionException e) { // TODO Auto-generated catch block e.printStackTrace(); } catch (InterruptedException e) { // TODO Auto-generated catch block e.printStackTrace(); } pool.shutdown(); return null; }

yes, but no, because the Callable depends on the client and on the surv.
My point is pass in the stuff that changes. The idea still holds.
not really, because the itself callable changes (to make it not change, it should not be just a callable, but also be able to have parameters for client and surv).
Give the Callable what it needs as parameters. Can still do it my way.

Collectives™ on Stack Overflow

How can I factorise this code? [duplicate]

3 Answers 3

Comments

Comments

8 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

8 Comments

Linked

Related