patternjavaMinor
Make the awt.Color class constructors except out of range arguments
Viewed 0 times
constructorstheargumentsawtrangecolormakeclassexceptout
Problem
Ok, this might be a very stupid question/idea (I'm still a beginner). Lets say I have a construct like this:
Now it is very likely that the passed arguments will in some cases be out of the expected range, so the code will throw an exception. To circumvent this case I wrote the following helper class:
Now I can do this:
I guess the question is, does that make any sense? The whole thing looks a bit overdone to me. Is there a better/easier way to accomplish the same thing?
Edit: For anyone interested. I boiled it down to this:
```
public class SafeColor {
private static final float FLOAT_MAX = 1;
private static final float FLOAT_MIN = 0;
private static final int INT_MAX = 255;
private static final int INT_MIN = 0;
private static float safeFloat(float f) {
return Math.max(Math.min(f, FLOAT_MAX), FLOAT_MIN);
}
private static int safeInt(int i) {
return Math.max(Math.min(i, INT_MAX), INT_MIN);
}
public static Color safeColor(float r, float g, float b, float a) {
return new Color(safeFloat(r), safeFloat(g), safeFloat(b), safeFloat(a));
}
public static Color safeColor(float r, float g, float b) {
return new Color(safeFloat(r), safeFloat(g), safeFloat(b));
}
g.setColor(new Color(random.nextInt(5) * speed, 255, speed * 3));Now it is very likely that the passed arguments will in some cases be out of the expected range, so the code will throw an exception. To circumvent this case I wrote the following helper class:
public class SafeColor {
static final int INT_MIN = 0;
static final int INT_MAX = 255;
static final float FLOAT_MIN = 0;
static final float FLOAT_MAX = 1;
public static Color safeIntColor(int r, int g, int b) {
if (r INT_MAX) r = INT_MAX;
if (g > INT_MAX) g = INT_MAX;
if (b > INT_MAX) b = INT_MAX;
return new Color(r, g, b);
}
public static Color safeIntAlphaColor(int r, int g, int b, int a) {
if (r INT_MAX) r = INT_MAX;
if (g > INT_MAX) g = INT_MAX;
// ... and so on ...Now I can do this:
g.setColor(SafeColor.safeIntColor(random.nextInt(5) * speed, 255, speed * 3));I guess the question is, does that make any sense? The whole thing looks a bit overdone to me. Is there a better/easier way to accomplish the same thing?
Edit: For anyone interested. I boiled it down to this:
```
public class SafeColor {
private static final float FLOAT_MAX = 1;
private static final float FLOAT_MIN = 0;
private static final int INT_MAX = 255;
private static final int INT_MIN = 0;
private static float safeFloat(float f) {
return Math.max(Math.min(f, FLOAT_MAX), FLOAT_MIN);
}
private static int safeInt(int i) {
return Math.max(Math.min(i, INT_MAX), INT_MIN);
}
public static Color safeColor(float r, float g, float b, float a) {
return new Color(safeFloat(r), safeFloat(g), safeFloat(b), safeFloat(a));
}
public static Color safeColor(float r, float g, float b) {
return new Color(safeFloat(r), safeFloat(g), safeFloat(b));
}
Solution
There is nothing too bad about this, and it is an acceptable pattern to get rid of extra/boilerplate code (for example
However, I would argue that it is better to validate your inputs to make sure that they don't exceed the ranges, or at least change the code that generates your values to ensure that they do not generate invalid values.
For example, you could have a method called
I would also suggest using
IOUtils from Apache Commons has a closeQuietly for streams that takes a way a lot of the nested try-catch-finally madness for closing streams).However, I would argue that it is better to validate your inputs to make sure that they don't exceed the ranges, or at least change the code that generates your values to ensure that they do not generate invalid values.
For example, you could have a method called
generateColor(speed) that returns a sane Color instance.I would also suggest using
Math.max and Math.min in this method instead of using a bunch of ifs.Context
StackExchange Code Review Q#29316, answer score: 5
Revisions (0)
No revisions yet.