From: Jim Janney on
Tom Anderson <twic(a)urchin.earth.li> writes:

> On Thu, 19 Nov 2009, Jim Janney wrote:
>
>> Tom Anderson <twic(a)urchin.earth.li> writes:
>>
>>> class FooMangler implements Mangler {
>>> static {
>>> ManglerRegistry.register("foo", FooMangler.class);
>>> }
>>> }
>>
>> To me what smells here is that ManglerRegistry is using static
>> methods. I'd prefer creating an instance of ManglerRegistry,
>> registering the appropriate manglers with it, and keeping a
>> reference to it in the ManglingParser. This requires more coding,
>> obviously, but gives you more flexibility in that different parsers
>> can use different registries.
>
> You're absolutely right, of course, and if i was in charge, that's how
> it would be done. Sadly, the ManglerRegistry is supplied by someone
> else, their parsing code depends on it, and we need their parsing
> code. All very annoying.

Ah. In that case, I'd probably just get rid of the static blocks and
do the registration directly in your initializer: that is, change

void initialise() {
Class.forName(Foo.class.getName());
}

to

void initialise() {
ManglerRegistry.register("foo", FooMangler.class);
}


Since you have to reference each class in the initializer anyway, it
doesn't look as if the static blocks are buying you anything.

--
Jim Janney
From: Lew on
Jim Janney wrote:
> Ah.  In that case, I'd probably just get rid of the static blocks and
> do the registration directly in your initializer: that is, change
>
> void initialise() {
>         Class.forName(Foo.class.getName());
>
> }
>
> to
>
> void initialise() {
>         ManglerRegistry.register("foo", FooMangler.class);
>

Did you mean'Foo.class'?

> }
>
> Since you have to reference each class in the initializer anyway, it
> doesn't look as if the static blocks are buying you anything.
>

As pointed out upthread a couple of times, merely referencing the
'class' literal will not initialize the 'FooMangler' class. Since
tom's goal is to initialize the classes that are being registered,
your suggestion will not work unless the 'ManglerRegistry.register()'
call invokes a method in 'FooMangler' or does one of the other things
that initializes the class, some of which have been suggested in this
conversation already.

--
Lew
From: Dangling Pointer on
On Nov 19, 6:12 pm, Lew <l...(a)lewscanon.com> wrote:
> Jim Janney wrote:
> > Ah. In that case, I'd probably just get rid of the static blocks and
> > do the registration directly in your initializer: that is, change
>
> > void initialise() {
> > Class.forName(Foo.class.getName());
>
> > }
>
> > to
>
> > void initialise() {
> > ManglerRegistry.register("foo", FooMangler.class);
>
> Did you mean'Foo.class'?
>
> > }
>
> > Since you have to reference each class in the initializer anyway, it
> > doesn't look as if the static blocks are buying you anything.
>
> As pointed out upthread a couple of times, merely referencing the
> 'class' literal will not initialize the 'FooMangler' class. Since
> tom's goal is to initialize the classes that are being registered

The goal was to register the classes. Tom wanted to initialize the
classes because doing so would make them self-register, but if they
can be registered directly, so much the better (as long as
registration is idempotent, so subsequent initialization of the
classes doesn't cause some sort of problems via "double
registration").

Of course, this depends on the apparently-closed-source API he's using
exposing a manual registration method, as well as on idempotency of
registration.
From: Arne Vajhøj on
Tom Anderson wrote:
> On Wed, 18 Nov 2009, Arne Vajh?j wrote:
>> Tom Anderson wrote:
>>> Am i right in thinking that all of these will force loading of Foo?
>>> Does anyone have any other idioms? How about any opinions on which
>>> idiom is best, or at least most idiomatic?
>>
>> I don't think the problem is common enough to make a solution idiomatic.
>
> It seems not.
>
>> I would without thinking twice:
>> - read class names from config file
>> - use Class.forName to initialize them
>
> The set of classes that need loading is very much static and code-like;
> it isn't the kind of thing that *needs* configuration at runtime. Given
> that, is reading a file really the right thing to do? How about putting
> the names in a string array?

I would use the file.

If your code will be used in 20 years, then I think there
is a decent chance that someone will want to reconfigure
at some point in time.

Arne
From: Jim Janney on
Dangling Pointer <dpointer2(a)gmail.com> writes:

> On Nov 19, 6:12 pm, Lew <l...(a)lewscanon.com> wrote:
>> Jim Janney wrote:
>> > Ah. In that case, I'd probably just get rid of the static blocks and
>> > do the registration directly in your initializer: that is, change
>>
>> > void initialise() {
>> > Class.forName(Foo.class.getName());
>>
>> > }
>>
>> > to
>>
>> > void initialise() {
>> > ManglerRegistry.register("foo", FooMangler.class);
>>
>> Did you mean'Foo.class'?
>>
>> > }
>>
>> > Since you have to reference each class in the initializer anyway, it
>> > doesn't look as if the static blocks are buying you anything.
>>
>> As pointed out upthread a couple of times, merely referencing the
>> 'class' literal will not initialize the 'FooMangler' class. Since
>> tom's goal is to initialize the classes that are being registered
>
> The goal was to register the classes. Tom wanted to initialize the
> classes because doing so would make them self-register, but if they
> can be registered directly, so much the better (as long as
> registration is idempotent, so subsequent initialization of the
> classes doesn't cause some sort of problems via "double
> registration").
>
> Of course, this depends on the apparently-closed-source API he's using
> exposing a manual registration method, as well as on idempotency of
> registration.

Exactly. I'm assuming that

ManglerRegistry.register(String tag, Class<? extends Mangler> manglerClass)

(or whatever) simply sticks the class into a hashmap until the XML
parser encounters the appropriate tag, whereupon an instance of the
mangler is created through reflection. I've written similar code for
other SAX-based parsers. This approach is arguably superior in that
the class isn't initialized until it's actually needed -- which may
not happen at all.

--
Jim Janney