2

This code works fine, but I need to simplify it for more clearness and hopefully more efficiency:

int i = 0; if (p.cap()) n++; if (p.creditcard()) n++; if (p.email()) n++; [...] if (p.price()) n++; if (p.url()) n++; if (p.zip()) n++; if (n == 0) p.standard();

As the code says, I need to call multiple methods (I don't know the finite number of them). Every p.()* method returns a boolean value, and n is incremented only if the value returned is true. If n==0 (this happens when EVERY method called returns false) then I need to call p.standard().

How can I write a more clear and efficient code? I tried with the or condition, something like this:

if (!( p.cap() || p.email() || p.isbn() || p.number() || p.phone() || p.price() || p.time() || p.url() || p.zip() || p.creditcard() )) { p.standard(); }

But obviously it didn't work properly (example: if p.cap() returns true the other methods are not called).

I need to call every method.

Improve this question
7
  • 1
    maybe: codereview.stackexchange.com? Commented Jun 28, 2012 at 11:58
  • 4
    The java compiler usually does a lot of optimization, so I would keep a piece of code like this rather readable than short. Commented Jun 28, 2012 at 12:01
  • Why do you say that the second example doesn't work properly? If the other methods are not called, isn't this the optimization you are expecting? Commented Jun 28, 2012 at 12:04
  • @OtavioMacedo It doesn't work properly because I need to call also the other methods. Commented Jun 28, 2012 at 12:05
  • 1
    If you want to migrate, just flag and ask a mod to do it. Please don't crosspost. Commented Jun 28, 2012 at 14:18

4 Answers 4

Reset to default
6

You did not specify if every method has to be called, but it seems you want to call them all regardless of individual results. So use the simple or operator: | (not the short circuit or ||).

if (!( p.cap() | p.email() | p.isbn() | p.number() | p.phone() | p.price() | p.time() | p.url() | p.zip() | p.creditcard() )) { p.standard(); }
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

3

With some boilerplate, you could abstract this into some sort of validator interface: 

interface Validator { boolean validate(Foo p); } Validator[] validators = new Validator[] { new Validator() { boolean validate(Foo p) {return p.cap();} }, new Validator() { boolean validate(Foo p) {return p.creditcard ();} }, new Validator() { boolean validate(Foo p) {return p.email();} }, // … } public int validateAll(Foo p, Validator[] validators) { int valid = 0; for (Validator v : validators) { if (v.validate(p)) valid++; } return valid; } if (validateAll(p, validators)) p.standard();

This is a net increase in code, but it has the advantage of communicating "run all these checks on p" clearly, and the list of checks is extensible.

(I admit this might easily be a solution that's way too heavy for your needs.)

Improve this answer

Comments

2

really hard to do here - not enough context...

but make a new method on the p object that returns the value you are looking for... then call it from the place where all this code is now.

something like 

int n = p.getPopulatedColumns();

then inside that method, it doesnt matter that much what the implementation is - because the reader will know the intent.

Improve this answer

Comments

2

There is another quite elegant solution IMO.

Create a validate method like this:

public static int validate(boolean ... booleans) { int n = 0; for (boolean b : booleans) { if (b) n++; } return n; }

Then you can call this method like this:

int n = validate(p.cap(), p.creditcard(), p.email()); if (n == 0) p.standard();

Since the validate method takes the booleans as variadics you can add as many (or as few) arguments as you want.

Or maybe you can simplify to return a boolean if all arguments are false:

public static boolean validate(boolean ... booleans) { int n = 0; for (boolean b : booleans) { if (b) n++; } return 0 == n; }

That all depends on if you need the n variable later on or not.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.