Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #36693 > unrolled thread

How to handle same sanity checks

Back to article view | Back to comp.lang.java.programmer

Contents

  How to handle same sanity checks Cecil Westerhof <Cecil@decebal.nl> - 2017-06-22 11:10 +0200
    Re: How to handle same sanity checks Martin Gregorie <martin@address-in-sig.invalid> - 2017-06-22 10:27 +0000
      Re: How to handle same sanity checks Cecil Westerhof <Cecil@decebal.nl> - 2017-06-22 13:21 +0200
      Re: How to handle same sanity checks Cecil Westerhof <Cecil@decebal.nl> - 2017-06-23 06:51 +0200
      Re: How to handle same sanity checks Robert Klemme <shortcutter@googlemail.com> - 2017-06-24 12:06 +0200
        Re: How to handle same sanity checks Martin Gregorie <martin@address-in-sig.invalid> - 2017-06-24 10:33 +0000
          Re: How to handle same sanity checks Robert Klemme <shortcutter@googlemail.com> - 2017-06-24 16:46 +0200
            Re: How to handle same sanity checks Martin Gregorie <martin@address-in-sig.invalid> - 2017-06-24 19:37 +0000
    Re: How to handle same sanity checks Andreas Leitgeb <avl@logic.at> - 2017-06-22 11:26 +0000
      Re: How to handle same sanity checks Eric Sosman <esosman@comcast-dot-net.invalid> - 2017-06-22 07:44 -0400
        Re: How to handle same sanity checks Andreas Leitgeb <avl@logic.at> - 2017-06-22 12:30 +0000
    Re: How to handle same sanity checks Eric Sosman <esosman@comcast-dot-net.invalid> - 2017-06-22 07:40 -0400
      Re: How to handle same sanity checks v_borchert@despammed.com (Volker Borchert) - 2017-06-22 18:33 +0000
        Re: How to handle same sanity checks Cecil Westerhof <Cecil@decebal.nl> - 2017-06-23 06:48 +0200
          Re: How to handle same sanity checks Eric Sosman <esosman@comcast-dot-net.invalid> - 2017-06-23 09:57 -0400
            Re: How to handle same sanity checks Cecil Westerhof <Cecil@decebal.nl> - 2017-06-23 16:31 +0200
              Re: How to handle same sanity checks Martin Gregorie <martin@address-in-sig.invalid> - 2017-06-23 17:21 +0000
                Re: How to handle same sanity checks Eric Douglas <e.d.programmer@gmail.com> - 2017-06-26 05:21 -0700
                  Re: How to handle same sanity checks Martin Gregorie <martin@address-in-sig.invalid> - 2017-06-26 13:34 +0000
                    Re: How to handle same sanity checks Martin Gregorie <martin@address-in-sig.invalid> - 2017-06-26 15:22 +0000
          Re: How to handle same sanity checks Arne Vajhøj <arne@vajhoej.dk> - 2017-06-25 22:55 -0400
    Re: How to handle same sanity checks Marcel Mueller <news.5.maazl@spamgourmet.org> - 2017-06-22 23:07 +0200
    Re: How to handle same sanity checks Joerg Meier <joergmmeier@arcor.de> - 2017-06-25 09:52 +0200
    Re: How to handle same sanity checks Robert Klemme <shortcutter@googlemail.com> - 2017-06-25 12:31 +0200
      Re: How to handle same sanity checks Arne Vajhøj <arne@vajhoej.dk> - 2017-06-25 14:23 -0400
        Style police (Was: Re: How to handle same sanity checks) Eric Sosman <esosman@comcast-dot-net.invalid> - 2017-06-25 17:28 -0400
          Re: Style police (Was: Re: How to handle same sanity checks) Arne Vajhøj <arne@vajhoej.dk> - 2017-06-25 19:14 -0400
          Re: Style police (Was: Re: How to handle same sanity checks) Arne Vajhøj <arne@vajhoej.dk> - 2017-06-25 19:19 -0400
            Re: Style police (Was: Re: How to handle same sanity checks) Robert Klemme <shortcutter@googlemail.com> - 2017-06-26 23:34 +0200

Page 2 of 2 — ← Prev page 1 [2]

#36725

On 6/23/2017 12:48 AM, Cecil Westerhof wrote:
> On Thursday 22 Jun 2017 20:33 CEST, Volker Borchert wrote:
>> Eric Sosman wrote:
>>> On 6/22/2017 5:10 AM, Cecil Westerhof wrote:
>>>> In my code I have several methods that start with:
>>>> if ((n < MIN_VAL) || (n > MAX_VAL)) {
>>>> throw new InvalidParameterException(String.format(ERROR_N, n));
>>>> }
>>>>
>>>> That is not really DRY.
>>>>
>>>> I could write a method like:
>>>> private void checkN(n) {
>>>> if ((n < MIN_VAL) || (n > MAX_VAL)) {
>>>> throw new InvalidParameterException(String.format(ERROR_N, n));
>>>> }
>>>> }
>>>>
>>>> And then replace all the checks with a call to checkN. But then
>>>> the exception is generated in the method checkN instead of the
>>>> method that has a wrong parameter. What is the best way to handle
>>>> this?
>>>
>>> Tastes surely vary, but for repetitive checks I favor writing the
>>> private internal helper method. Yes, the caller of doItPlease()
>>> gets an exception originating from internalSanityCheck() instead,
>>> but that shouldn't bother anyone. doItPlease() will be in the stack
>>> trace anyhow, even though it's not at the very top.
>>
>> That can be a problem with some more-or-less-broken logging
>> frameworks, and a nuisance if trying to diagnose over the phone.
>>
>> You can always do something like
>>
>> 	final InvalidParameterException e
>> 	  = new InvalidParameterException(String.format(ERROR_N, n));
>> 	final StackTraceElement[] s = e.getStackTrace();
>> 	e.setStackTrace(Arrays.copyOfRange(s, 1, s.length));
>> 	throw e;
> 
> That is a very good idea. I have now:
>      private static void checkInRange(final int n,
>                                       final int lowLimit,
>                                       final int highLimit,
>                                       final String message) {
>          if ((n < lowLimit) || (n > highLimit)) {
>              final InvalidParameterException e =
>                  new InvalidParameterException(
>                      String.format(message, lowLimit, highLimit, n));
>              final StackTraceElement[] s = e.getStackTrace();
>              e.setStackTrace(Arrays.copyOfRange(s, 1, s.length));
>              throw e;
>          }
>      }
> 
> Works like a charm.

> The code is now more clear and easier to change, while the stack trace
> gives the right information.

That is probably as far as you can get in pure Java.

If you are willing to use AspectJ with annotations there is
another way.

Demo:

package paramcheck;

import java.util.function.Consumer;

public class TestMain {
	@CheckParams
	public static void test1(@IntIntervalCheck(minValue=1,maxValue=10) int v) {
		System.out.println(v + " is OK");
	}
	@CheckParams
	public static void test2(@StringLengthCheck(minLength=1,maxLength=10) 
@StringPatternCheck(pattern="[A-Z]\\d+") String s) {
		System.out.println(s + " is OK");
	}
	public static <T> void test(Consumer<T> f, T o) {
		try {
			f.accept(o);
		} catch(ParamException ex) {
			System.out.println(ex.getMessage());
			ex.printStackTrace();
		}
	}
	public static void main(String[] args) {
		test((o) -> test1(o), 5);
		test((o) -> test1(o), 50);
		test((o) -> test2(o), "A123");
		test((o) -> test2(o), "XXXXXXXXXXXXXXX");
		test((o) -> test2(o), "ABCD");
	}
}

Output:

5 is OK
50 not in interval
paramcheck.ParamException: 50 not in interval
         at paramcheck.TestMain.test1(TestMain.java:8)
         at paramcheck.TestMain.lambda$1(TestMain.java:24)
         at paramcheck.TestMain.test(TestMain.java:16)
         at paramcheck.TestMain.main(TestMain.java:24)
A123 is OK
XXXXXXXXXXXXXXX invalid length
paramcheck.ParamException: XXXXXXXXXXXXXXX invalid length
         at paramcheck.TestMain.test2(TestMain.java:12)
         at paramcheck.TestMain.lambda$3(TestMain.java:26)
         at paramcheck.TestMain.test(TestMain.java:16)
         at paramcheck.TestMain.main(TestMain.java:26)
ABCD does not match pattern
paramcheck.ParamException: ABCD does not match pattern
         at paramcheck.TestMain.test2(TestMain.java:12)
         at paramcheck.TestMain.lambda$4(TestMain.java:27)
         at paramcheck.TestMain.test(TestMain.java:16)
         at paramcheck.TestMain.main(TestMain.java:27)

(see the rest of the code below)

I think it is nice code. But it also has its drawbacks - it
brings in an entire new paradigm.

Arne

package paramcheck;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

@Retention(RetentionPolicy.RUNTIME)
public @interface IntIntervalCheck {
	public int minValue() default 0;
	public int maxValue() default Integer.MAX_VALUE;
	public String message() default "%d not in interval";
}

package paramcheck;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

@Retention(RetentionPolicy.RUNTIME)
public @interface StringLengthCheck {
	public int minLength() default 0;
	public int maxLength() default 255;
	public String message() default "%s invalid length";
}

package paramcheck;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

@Retention(RetentionPolicy.RUNTIME)
public @interface StringPatternCheck {
	public String pattern() default ".*";
	public String message() default "%s does not match pattern";
}

package paramcheck;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

@Retention(RetentionPolicy.RUNTIME)
public @interface CheckParams {
}

package paramcheck;

import java.lang.reflect.Method;

import org.aspectj.lang.JoinPoint;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.aspectj.lang.annotation.Pointcut;
import org.aspectj.lang.reflect.MethodSignature;

@Aspect
public class CheckParamsAdvice {
	@Pointcut("execution(@paramcheck.CheckParams * *.*(..))")
	public void checkParamsPointcut() { }
	@Before("checkParamsPointcut()")
	public void checkParams(JoinPoint jp) {
		try {
			Method m = ((MethodSignature)jp.getSignature()).getMethod();
			Object[] args = jp.getArgs();
			for(int i = 0; i < args.length; i++) {
				ParamsChecker.check(m.getParameterAnnotations()[i], args[i]);	
			}
		} catch (SecurityException e) {
			e.printStackTrace();
		}
	}
}

package paramcheck;

import java.lang.annotation.Annotation;
import java.util.Arrays;
import java.util.regex.Pattern;

public class ParamsChecker {
	public static void throwException(String msg) {
		ParamException ex = new ParamException(msg);
		StackTraceElement[] ste = ex.getStackTrace();
		ex.setStackTrace(Arrays.copyOfRange(ste, 3, ste.length));
		throw ex;
	}
	public static void check(Annotation[] ans, Object o) {
		for(Annotation an : ans) {
			if(an instanceof IntIntervalCheck) {
				IntIntervalCheck iic = (IntIntervalCheck)an;
				int v = (int)o;
				if(v < iic.minValue() || iic.maxValue() < v) {
					throwException(String.format(iic.message(), v));
				}
			}
			if(an instanceof StringLengthCheck) {
				StringLengthCheck slc = (StringLengthCheck)an;
				String s = (String)o;
				if(s.length() < slc.minLength() || slc.maxLength() < s.length()) {
					throwException(String.format(slc.message(), s));
				}
			}
			if(an instanceof StringPatternCheck) {
				StringPatternCheck spc = (StringPatternCheck)an;
				String s = (String)o;
				if(!Pattern.matches(spc.pattern(), s)) {
					throwException(String.format(spc.message(), s));
				}
			}
		}
	}
}

package paramcheck;

public class ParamException extends RuntimeException {
	private static final long serialVersionUID = 1L;
	public ParamException(String msg) {
		super(msg);
	}
}

[toc] | [prev] | [next] | [standalone]

#36701

On 22.06.17 11.10, Cecil Westerhof wrote:
> I could write a method like:
>      private void checkN(n) {
>          if ((n < MIN_VAL) || (n > MAX_VAL)) {
>              throw new InvalidParameterException(String.format(ERROR_N, n));
>          }
>      }
>
> And then replace all the checks with a call to checkN. But then the
> exception is generated in the method checkN instead of the method that
> has a wrong parameter.
> What is the best way to handle this?

Besides just accepting the difference as recommended you may write code 
in checkN that removes the last stack frame from the stack trace before 
throwing the exception. Use setStackTrace for this purpose.


Marcel

[toc] | [prev] | [next] | [standalone]

#36718

On Thu, 22 Jun 2017 11:10:38 +0200, Cecil Westerhof wrote:

> In my code I have several methods that start with:
>     if ((n < MIN_VAL) || (n > MAX_VAL)) {
>         throw new InvalidParameterException(String.format(ERROR_N, n));
>     }

> That is not really DRY.

May I suggest taking a look at Guavas Preconditions ?

checkArgument(n < MIN, "Expected n > MIN, but %s > %s", n, MIN);
checkArgument(n > MAX, "Expected n < MAX, but %s < %s", n, MAX);

Liebe Gruesse,
		Joerg

-- 
Ich lese meine Emails nicht, replies to Email bleiben also leider
ungelesen.

[toc] | [prev] | [next] | [standalone]

#36720

On 22.06.2017 11:10, Cecil Westerhof wrote:
> In my code I have several methods that start with:
>      if ((n < MIN_VAL) || (n > MAX_VAL)) {
>          throw new InvalidParameterException(String.format(ERROR_N, n));
>      }
> 
> That is not really DRY.

A few minor remarks: brackets around comparisons are not needed.  Is 
this really part of JCA code?  If not I would just use 
IllegalArgumentException to not give a false impression.

> I could write a method like:
>      private void checkN(n) {
>          if ((n < MIN_VAL) || (n > MAX_VAL)) {
>              throw new InvalidParameterException(String.format(ERROR_N, n));
>          }
>      }
> 
> And then replace all the checks with a call to checkN. But then the
> exception is generated in the method checkN instead of the method that
> has a wrong parameter.
> What is the best way to handle this?

I would not worry about the stack too much (see other subthreads of this 
discussion).  Going through all the hassle of removing an entry just to 
let the stack trace end up one level up is way too much effort here. 
After all, we are talking about a RuntimeException that is intended to 
catch a programmer error, likely during testing.  In that context there 
is usually no issue having logging switched on to a level of verbosity 
to see the stack trace.

I would suggest another name for the method though, more along the lines 
of ensureArgumentIsInRange(final int n).  A "check" can be anything and 
result in anything (ask yourself "what is the result of the check?") but 
what you really want here is that processing only continues if the 
values are within the range.

Kind regards

	robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/

[toc] | [prev] | [next] | [standalone]

#36721

On 6/25/2017 6:31 AM, Robert Klemme wrote:
> On 22.06.2017 11:10, Cecil Westerhof wrote:
>> In my code I have several methods that start with:
>>      if ((n < MIN_VAL) || (n > MAX_VAL)) {
>>          throw new InvalidParameterException(String.format(ERROR_N, n));
>>      }
>>
>> That is not really DRY.
> 
> A few minor remarks: brackets around comparisons are not needed.

Not necessary.

But actually recommended in the traditional Java coding convention.

<quote>
10.5.1 Parentheses

It is generally a good idea to use parentheses liberally in expressions 
involving mixed operators to avoid operator precedence problems. Even if 
the operator precedence seems clear to you, it might not be to 
others-you shouldn't assume that other programmers know precedence as 
well as you do.

     if (a == b && c == d)     // AVOID!
     if ((a == b) && (c == d)) // RIGHT

</quote>

Arne

[toc] | [prev] | [next] | [standalone]

#36722 — Style police (Was: Re: How to handle same sanity checks)

On 6/25/2017 2:23 PM, Arne Vajhøj wrote:
> On 6/25/2017 6:31 AM, Robert Klemme wrote:
>> [...]
>> A few minor remarks: brackets around comparisons are not needed.
> 
> Not necessary.
> 
> But actually recommended in the traditional Java coding convention.
> 
> <quote>
> 10.5.1 Parentheses
> 
> It is generally a good idea to use parentheses liberally in expressions 
> involving mixed operators to avoid operator precedence problems. Even if 
> the operator precedence seems clear to you, it might not be to 
> others-you shouldn't assume that other programmers know precedence as 
> well as you do.
> 
>      if (a == b && c == d)     // AVOID!
>      if ((a == b) && (c == d)) // RIGHT
> 
> </quote>

     Is anybody aware of any actual research into the value of
this "generally good" practice, or does "generally good" merely
mean "it's my prejudice, so as author I'll claim it's good?"

     I'll note in passing that the author is no Java Jedi.  Consider
section 10.4 of the same document, which recommends

	if ((c++ = d++) != 0)

... as an "improvement" over an equally uncompilable bad example.

-- 
esosman@comcast-dot-net.invalid
Thirteen hundred five days to go.

[toc] | [prev] | [next] | [standalone]

#36723 — Re: Style police (Was: Re: How to handle same sanity checks)

On 6/25/2017 5:28 PM, Eric Sosman wrote:
> On 6/25/2017 2:23 PM, Arne Vajhøj wrote:
>> On 6/25/2017 6:31 AM, Robert Klemme wrote:
>>> [...]
>>> A few minor remarks: brackets around comparisons are not needed.
>>
>> Not necessary.
>>
>> But actually recommended in the traditional Java coding convention.
>>
>> <quote>
>> 10.5.1 Parentheses
>>
>> It is generally a good idea to use parentheses liberally in 
>> expressions involving mixed operators to avoid operator precedence 
>> problems. Even if the operator precedence seems clear to you, it might 
>> not be to others-you shouldn't assume that other programmers know 
>> precedence as well as you do.
>>
>>      if (a == b && c == d)     // AVOID!
>>      if ((a == b) && (c == d)) // RIGHT
>>
>> </quote>
> 
>      Is anybody aware of any actual research into the value of
> this "generally good" practice, or does "generally good" merely
> mean "it's my prejudice, so as author I'll claim it's good?"

Many of these best practices are very hard to empirically
prove.

So it end up with whether you trust the people preaching
the best practice.

This one is also in McConnell's Code Complete.

Section 31.2:

<quote>
Use more parentheses than you think you need. Use parentheses to
clarify expressions that involve more than two terms. They may
not be needed, but they add clarity and they don't cost you anything.
</quote>

I have noticed that it is a very common problem among
beginner programmers to get unexpected results due to lack
of parentheses. For whatever reason SQL seems particular
prone to this error.

Whether you would let your coding style influence by:
* The gentleman Peter King, Patrick Naughton, Mike DeMoney,
   Jonni Kanerva, Kathy Walrath, and Scott Hommel from SUN
   (that apperently contributed to Java coding convention)
* McConnel
* a random dude on the internet: me
is up to you.

Arne

[toc] | [prev] | [next] | [standalone]

#36724 — Re: Style police (Was: Re: How to handle same sanity checks)

On 6/25/2017 5:28 PM, Eric Sosman wrote:
> On 6/25/2017 2:23 PM, Arne Vajhøj wrote:
>> On 6/25/2017 6:31 AM, Robert Klemme wrote:
>>> [...]
>>> A few minor remarks: brackets around comparisons are not needed.
>>
>> Not necessary.
>>
>> But actually recommended in the traditional Java coding convention.
>>
>> <quote>
>> 10.5.1 Parentheses
>>
>> It is generally a good idea to use parentheses liberally in 
>> expressions involving mixed operators to avoid operator precedence 
>> problems. Even if the operator precedence seems clear to you, it might 
>> not be to others-you shouldn't assume that other programmers know 
>> precedence as well as you do.
>>
>>      if (a == b && c == d)     // AVOID!
>>      if ((a == b) && (c == d)) // RIGHT
>>
>> </quote>
> 
>      Is anybody aware of any actual research into the value of
> this "generally good" practice, or does "generally good" merely
> mean "it's my prejudice, so as author I'll claim it's good?"

Also note that this particular item in the standard Java coding
convention is not one that I would consider worth pointing
out if I saw a code not following - it is within what I consider
"acceptable deviations".

So absolutely no "style police" here.

I will though point to it if somebody suggests the
opposite convention.

"style police police" or whatever.

Arne

[toc] | [prev] | [next] | [standalone]

#36737 — Re: Style police (Was: Re: How to handle same sanity checks)

On 26.06.2017 01:19, Arne Vajhøj wrote:
> On 6/25/2017 5:28 PM, Eric Sosman wrote:
>> On 6/25/2017 2:23 PM, Arne Vajhøj wrote:
>>> On 6/25/2017 6:31 AM, Robert Klemme wrote:
>>>> [...]
>>>> A few minor remarks: brackets around comparisons are not needed.
>>>
>>> Not necessary.
>>>
>>> But actually recommended in the traditional Java coding convention.

> I will though point to it if somebody suggests the
> opposite convention.

Which I didn't do.  Just for the record...

Cheers

	robert

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | comp.lang.java.programmer

csiph-web