In general, I think your code looks fine. It looks like how you would
write things if it were a Swing app.
A possible improvement is to rely more on the request-response nature
of Wicket. In desktop applications, you are generally in charge of
letting dependent components know some state has changed, and you
typically construct a network of property listeners to propogate such
changes. You can do the same with Wicket, but alternatively you can
make use of the knowledge that when a request comes in, the whole set
of components will be visited (for rendering), including the models
etc. If you use that knowledge, you can invert the strategy from
pushing changes to interested listeners, to models just pulling the
their state just in time.
For example, instead of:
private final ArrayList<String> listSnippetIds = new ArrayList<String>();
private void updateSnippetList() {
listSnippetIds.clear();
JpoxDataAccess da = new JpoxDataAccess();
for (Object objCurSnippet : da.getBoExtent(HtmlSnippet.class))
listSnippetIds.add(((HtmlSnippet)
objCurSnippet).getDatastoreId());
}
you could have a model like this:
private static final class SnippetListModel extends LoadableDetachableModel {
protected abstract Object load() {
List<HtmlSnippet> list = new ArrayList<HtmlSnippet>()
JpoxDataAccess da = new JpoxDataAccess();
for (Object objCurSnippet : da.getBoExtent(HtmlSnippet.class)) {
list.add((HtmlSnippet) objCurSnippet);
}
return list;
}
}
frmEdit.add(new ListChoice("selectSnippets", new SnippetListModel()));
You don't need the instance variable snippet ids then. Also you can
work with the complete objects instead of id's while even being more
efficient then before. This is because LoadableDetachableModel is a
detachable model, meaning that on first use it will be attached (in
this case will have the effect that method 'load' is called, and the
result of load is set as a temporary, transient value of the model)
and after the request is done, the model will be detached (in this
will have the effect that the temporary var - the result of load - is
set to null again, so that next time getObject is called, the model
will be attached again first).
So what we achieved here is that you always work with the objects as
fresh as they get, while nothing is stored as page state except for
the model instance itself; during a request the snippets are loaded
from the database, and at the end of the request, the snippets are
removed again.
Another thing we achieved here is that we now have a completely
standalone model. As there is no dependency with the page, you could
reuse this model anywhere you need a list of snippets. There are
endless variations of course, including having full-fledged models
that take parameters for filtering, sorting etc.
A similar thing can be said about the snippetBeingEdited variable.
Everyone should be very careful with storing persistent objects (JDO,
Hibernate, EJB objects) as page state. In most cases you shouldn't do
that, as it'll give you problems with lazy collections, the
persistence engine's session (like Hibernate's Session) they are
supposed to be part of, the transaction they might take part of, etc.
It is much better to use detachable models here too. So typically, you
would have a model that stores the id of the persistent object you
want to use, and when attached, you would load that object, and when
detached you would release it again. A specialization of
LoadableDetachableModel could work here, or e.g. extend
AbstractReadOnlyDetachableModel. Typically - e.g. with Hibernate - if
you share objects in the same session/ transaction they will be hold
in a session cache, so unless you get your objects using queries your
persistence engine don't know how to cache, using seperate detachable
models for such database objects is efficient enough.
Finally, about using annonymous classes. Typically, this is a
tradeoff. Annonynmous classes make for a very compact writing style,
and in cases like writing links and buttons etc where you just
implement the event handler, using annonymous classes usually makes
the code better readable and quicker to write. The other side however
is that explicit classes are easier to refactor. Also, if you start
writing them as static or top level classes, only to revert to
non-static private classes (which can use member variables of the
parent classes) when it really makes sense, you'll have a much clearer
idea of what will be component state (and thus be stored in the
session) and what not.
Hope this helps,
Eelco
> I know it's a lot of code, but I would greatly appreciate it if someone here
> could skim through it and point out anything I'm doing that will bite me in
> the ass later on down the road. In particular, I wonder about:
>
> Do people generally just use anonymous inner classes for Button event
> handling or write fully-subclassed private inner classes?
> To track the current state of things, I use a non-final HtmlSnippet member
> variable and a separate model class for the entered values. Does this
> dual-set up make sense?
> In order for my Panel to notify the enclosing WebPage instance that the
> database has been updated, I wrote a simple listener for the Panel. Is
> there a way to do this that makes more sense since I have both the list of
> objects and the editor on the same page?
_______________________________________________
Wicket-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-user
