Hi,

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().

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()});
(Yes, I also would like slf4j to use varargs from Java 5.)
Follow this link for more info.
http://www.slf4j.org/faq.html#logging_performance
http://www.slf4j.org/faq.html#logging_performance 

Best regards,
Peter

-- 
View this message in context: 
http://www.nabble.com/some-remarks-on-LoadableDetachableModel.getObject%28%29-tf4313719.html#a12281762
Sent from the Wicket - Dev mailing list archive at Nabble.com.

Reply via email to