> I'm new to wicket so I'm still learning from the source.
> So I came across this method in LoadableDetachableModel
>
> public Object getObject()
> {
> if (!attached)
> {
> attached = true;
> transientModelObject = load();
>
> if (log.isDebugEnabled())
> {
> log.debug("loaded transient object " +
> transientModelObject + " for " +
> this
> + ", requestCycle " +
> RequestCycle.get());
> }
>
> onAttach();
> }
> return transientModelObject;
> }
>
> IMHO there two things that might be improved.
>
> The flag attached is set before doing the actual job. This is not exception
> safe. As load() is an abstract method, it might be implemented to do
> anything. If it throws an exception, attached has been set but
> transientModelObject is left in an unknown state. Next time this method is
> called, it will return some garbage.
> I think it will be better if attached is set after the cal to load().
We can consider that, though I wonder whether it is an actual problem
since if an exception occurs (mind you: runtime exception), the
component/ page shouldn't be rendered, period. If it should, you would
be misusing exceptions.
> The other thing is the use of log.isDebugEnabled(). It seems to be employed
> in quite number of places in wicket. I guess the idea is to improve
> performance. The same effect can be achieved in a simpler way, just do
> log.debug("loaded transient object {} for {}, requestCycle {}", new Object[]
> {transientModelObject, this, RequestCycle.get()});
It would perform slightly better when debugging is on, but a bit worse
if it is off. That single isDebugEnabled would still be cheaper than
letting log.debug demine whether the log level is on *and*
constructing the string and getting the request cycle.
Eelco
