2

I'm writing signal processing library,
and I want to make this kind of hierarchy.

First, I defined abstract class
that define only "this wrapper class has one double[] field"
i.e.

public abstract class DoubleArray{
    private final double[] array;
    DoubleArray(double[] array){
        this.array = array;
    }
}

(I made constructor package private rather than protected because
I want to restrict usage of constructor outside the package)

(System.arraycopy is not used here
because this constructor is used only by me.)

Next, my first restriction is,
"cannot contain exceptional values"
i.e. 

//inside the same package
public class SignalWithoutExceptionalValues extends DoubleArray{
    SignalWithoutExceptionalValues(double[] signal){
        super(signal);
        for(double d : signal)
            if(Double.isNaN(d) || Double.isInfinite(d))
                throw new IllegalArgumentException("cannot contain exceptional values");
    }
}

Next restriction is
"signal is in range of -1.0 ~ 1.0"
i.e. 

//inside the same package
public final class OneAmplitudeSignal extends SignalWithoutExceptionalValues{
    OneAmplitudeSignal(double[] signal){
        super(signal);
        for(double d : signal)
            if(d > 1.0 || d < -1.0)
                throw new IllegalArgumentException("is not normalized");
    }
}

However, I think,
first of all,
I have to do for-each argument checking,
and then, after that,
assign to field.

but in this example,
I am forced to assign unchecked array to field,
because DoubleArray constructor must initialize array field.

So, my question is,
is there any strategy to do "first checking then assign"
in this kind of hierarchy.
OR this design is not appropriate for this purpose,
and there is another good design ?

Thank you.

EDIT

thank you for first two answers.
but by those solution,
library user can instantiate "illegal" signal,
because "checking mechanism can be overrided" .

However, I perfer inheritance
because OneAmplitudeSignal object can be
assigned to SignalWithoutExceptionalValues variables.
This fact corresponds to OneAmp~ is subset of SignalWithout~.

Also, amplitude can be checked by someSignal instanceof OneAmplitudeSignal .

And "final" can't be used here
because I want to allow library users
to create new "more restricted signal"
or, "new kind of restriction" .

In addition, root class can be non-abstract .
in this case, "no restriction signal" can also be instantiated.

Improve this question
1
  • By the way, MinusOneToOneSignal is more appropriate. Is there more simple name? Commented Aug 31, 2016 at 15:24

2 Answers 2

Reset to default
1

You could extract the checking logic to a separate check method which the base constructor can execute prior to the assignment :

public abstract class DoubleArray{
    private final double[] array;
    protected abstract void check (double[] array);
    DoubleArray(double[] array){
        check(array);
        this.array = array;
    }
}

The sub-classes will override check to supply the required validation logic :

public class SignalWithoutExceptionalValues extends DoubleArray{
    SignalWithoutExceptionalValues(double[] signal){
        super(signal);
    }

    @Override
    protected void check (double[] signal) {
        for(double d : signal)
            if(Double.isNaN(d) || Double.isInfinite(d))
                throw new IllegalArgumentException("cannot contain exceptional values");
    }
}

public final class OneAmplitudeSignal extends SignalWithoutExceptionalValues{
    OneAmplitudeSignal(double[] signal){
        super(signal);
    }

    @Override
    protected void check (double[] signal) {
        super.check ();
        for(double d : signal)
            if(d > 1.0 || d < -1.0)
                throw new IllegalArgumentException("is not normalized");
    }
}
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

@ShuS Be ware that with this solution OneAmplitudeSignal is extending SignalWithoutExceptionalValues (and not DoubleArray) and requires that you don't forget to call super.check() (same applies if you have more rules) !
@Spotted I see . Then, this implementation can't be used , because I can't force library user to call super.check() . Is there another solution ? One thing I can think of is, to have boolean flag isNormalized , but I prefer inheritance because OneAmplitudeSignal is apparently subset of SignalWithout~ .
0

Since you are writing a library, it means that your library could be used in some cases you can't predict now. That's why I would avoid using inheritance because with inheritance you are scealing (enforcing) what you think is right in order to validate a signal by this static inheritance chain: OneAmplitudeSignal > SignalWithoutExceptionalValues > DoubleArray.

I would instead foster composition to do the validation. This is one possible implementation (more generic):

public interface Rule {
    DoublePredicate getPredicate();
    String getReason();
}

public final class ExceptionalValue implements Rule {
    @Override
    public DoublePredicate getPredicate() {
        return d -> Double.isNaN(d) || Double.isInfinite(d);
    }

    @Override
    public String getReason() {
        return "cannot contain exceptional values";
    }
}

public final class GreaterThanOneAmplitude implements Rule {
    @Override
    public DoublePredicate getPredicate() {
        return d -> d > 1.0 || d < -1.0;
    }

    @Override
    public String getReason() {
        return "is not normalized";
    }
}

public final class DoubleArray {
    private final double[] array;

    public DoubleArray(double[] signal, Collection<Rule> rules) {
        for (Rule rule : rules) {
            if (Arrays.stream(signal).anyMatch(rule.getPredicate())) {
                throw new IllegalArgumentException(rule.getReason());
            }
        }

        this.array = signal;
    }

    //...
}

On the other hand, if you don't need to (or want to) be generic simply do that:

public final class DoubleArray {
    private final double[] array;

    public DoubleArray(double[] signal) {
        for(double d : signal) {
            if(Double.isNaN(d) || Double.isInfinite(d)) {
                throw new IllegalArgumentException("cannot contain exceptional values");
            }
            if(d > 1.0 || d < -1.0) {
                throw new IllegalArgumentException("is not normalized");
            }
        }

        this.array = signal;
    }

    //...
}
Improve this answer

2 Comments

your implementation is smart, but I want to check amplitude by "someSignal instanceof OneAmplitudeSignal" .
@ShuS Not sure I understand correctly what you want to do with instanceof. Using instanceof is a sign of poor design and leads to maintanability problems.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.