Is it OK to use this pattern for value-checking?

Question

For my classes that have methods with similar input-checking logic (for example, a custom multi-dimensional array that has a lot of methods, all of which check if given coordinates are within the array limits), I create a separate private checker that throws runtime exceptions, and also a public checker, that just returns a boolean value indicating if a variable is acceptable for this class methods. Here's example:

public class Foo {



    public void doStuff(Variable v) {

        checkVariableUnsafe(v);
        ... // do stuff
    }



    private void checkVariableUnsafe(Variable v) throws InvalidVariableException {...}



    public boolean checkVariable(Variable v) {
        try {
            checkVariableUnsafe(v);
            return true;
        } catch (InvalidVariableException e) {
            return false;
        }
    }
}

Is it OK to use it, or are there some downfalls that I fail to see? What's the commonly used pattern in such situations?

Design patterns refer specifically to object level or class scope solutions, and solve problems associated with object creation, object interactions, and object communications. They are not functional scope solutions. I am retagging your question. — Sisyphus, Jul 04 '11 at 16:29
What's the source of your definition? According to wikipedia, design pattern isn't necessarily about objects. http://en.wikipedia.org/wiki/Design_pattern_(computer_science) — Max Yankov, Jul 04 '11 at 17:04
All programming contains patterns and all programming should contain design. The idea is not to define something but to keep topics within the range of what is a generally accepted concept. "Design patterns", a vague term, but there is a pretty clear idea of the context it usually refers to. The generally accepted concept most people understand as "Design Patterns" is based on objects and the basic categories of Creational, Structural and Behavioral patterns. All the major sources quoted in that wikipedia article - Fowler, Martin, Gang of Four etc are describing object based systems. — Sisyphus, Jul 05 '11 at 07:18

score 3 · Answer 1 · answered Jul 03 '11 at 13:09

It's not just a good idea to use the same code for both validity prediction and actual validation, it's the only right idea. And since the first commandment is Don't repeat yourself!, of course you should extract that check into a method of its own. So this is exactly what I usually do.

score 2 · Accepted Answer · edited May 23 '17 at 11:55

It is often recommended to avoid using exceptions for normal program flow. Without debating that issue here, if you wanted to follow that advice, then you could put the logic that actually does the check in the public checkVariable method, and have the private checkVariableUnsafe method call checkVariable and throw an exception if it returns false.

I don't think there is enough context in your question to comment definitively on the appropriateness of the API you have created, but I can see nothing intrinsically wrong with it.

score 1 · Answer 3 · answered Jul 03 '11 at 13:08

1

This is a very good practice. I'd just use a standard IllegalArgumentException rather than a custom one.

answered Jul 03 '11 at 13:08

JB Nizet

678,734
91
1,224
1,255

My custom exception extends IllegalArgumentException with custom logging information; I also find it useful for unit-tests that expect this exception to happen: this way they don't accidentally other, not expected IllegalArgumentExceptions. – Max Yankov Jul 03 '11 at 14:06

score 0 · Answer 4 · answered Jul 03 '11 at 13:07

0

I think this is unnecessarily complex. If you're just checking incoming parameters passed to a method, I'd just program in the checks, throw IllegalArgumentException if one fails, and move on. I'd rebel against anyone who imposed such a verbose, alleged pattern on me.

I would prefer something like Spring's binding and validation API, but that's not what you've got here. Your proposed complexity doesn't appear to buy me anything.

answered Jul 03 '11 at 13:07

duffymo

305,152
44
369
561

1

If the caller is able to know when an input is valid or not, I agree with you. If only Foo is able to know whether the input is valid or not, I don't see any better solution than this one. Just throwing an exception would force the caller to try calling the method and catch the IllegalArgumentException. This pattern is promoted by Josh Bloch, BTW. – JB Nizet Jul 03 '11 at 13:12
1

Why is it preferable to call a Boolean method before the setter, rather than call the method and catch any exception that results? The only time I can see this being useful is if you want to check and see if an input is valid prior to actually setting it, ie. Displaying a warning to a user if their preferred username is already taken. – Jesse Barnum Jul 03 '11 at 13:18
It's useful for exactly the reason you describe, and because it's bad practice to rely on exceptions for flow control. As PMD says (http://pmd.sourceforge.net/rules/strictexception.html#ExceptionAsFlowControl): Using Exceptions as flow control leads to GOTOish code and obscures true exceptions when debugging. This is also a recommendation of J. Bloch. See http://lcsd05.cs.tamu.edu/slides/keynote.pdf (slide 38) – JB Nizet Jul 03 '11 at 13:22
If you need to validate multiple business objects it is handy to have boolean methods rather than exceptions; it buys you more readable and compact validation code. – Adriaan Koster Jul 03 '11 at 13:38
Jesse, that's exactly the case. – Max Yankov Jul 03 '11 at 14:09

Is it OK to use this pattern for value-checking?

4 Answers4