Hi.

There are several problems.

There is a "protected" field ("generator").

There is a no-arg constructor that calls a overridable method
("makeGenerator()").  But the method is thus is called *before*
the subclass's constructor has run.

That method initializes the "generator" field.  If the object
returned by "makeGenerator()" needs initialization (performed by
its constructor), the returned instance will be invalid or "null"
or the call might just fail (raising an exception) in the attempt
to construct a generator instance with missing data.

This invalid object is then passed on to the constructor of
"RandomDataGenerator".

But "RandomDataGenerator" is a smart class: if the RNG is null,
it will create one and all the tests will still work; but they
won't use have used the RNG supposedly under test.

There are "@Before"-annotated methods, but it is not clear that
they are necessary.  In "RandomGeneratorAbstractTest", it partly
duplicates initialization performed by the constructor.

A number of test methods defined in "RandomDataGeneratorTest"
are overridden in "RandomGeneratorAbstractTest" in order to do
nothing; yet they are counted as "passed" in Junit's output.

Unit test "testSeeding" rely on an undefined behaviour in that
"makeGenerator()" will return a different instance at each call.

Reusing the same "generator" instance in several test is
deceiving, as Junit instantiates the class for each "@Test"
method.

All in all, I think that:
1. The "protected" instance variables should be abandoned.
2. The testing for "RandomDataGenerator" should be separate
   from the testing of the RNGs.


Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to