Skip to content
This repository has been archived by the owner on Apr 5, 2022. It is now read-only.

MOBILE-35 Added LiteDeviceDelegatingViewResolver #12

Merged
merged 2 commits into from Jan 14, 2013

Conversation

royclarkson
Copy link
Contributor

No description provided.

@royclarkson
Copy link
Contributor Author

For the following JIRA:

https://jira.springsource.org/browse/MOBILE-35

@royclarkson royclarkson mentioned this pull request Dec 19, 2012
@Override
protected void initServletContext(ServletContext servletContext) {
String name = delegate.getClass().getName();
getApplicationContext().getAutowireCapableBeanFactory().initializeBean(delegate, name);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should autowiring be done considering that the delegate was configured as an instance? That instance may have been (should have been) initialized already.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

err. no. that is an artifact of starting down one direction and changing course of the design.

@rstoyanchev
Copy link

I've added my comments.

@rstoyanchev
Copy link

One more potential simplification to consider is to remove the usePrefix/useSuffix boolean properties and rely on whether a specific prefix or suffix is provided or not (i.e. is not null). That removes the need for the abstract class, simplifies configuration, and also gives users full control over both the prefix and the suffix to use.

@royclarkson
Copy link
Contributor Author

The reason I established the default prefix and suffix is to simply the configuration, since you wouldn't have to specify it if you used the defaults. But as you suggest, you still have to specify one of the booleans, and its less clear how it is configured when using the defaults. Probably best to expect explicitly set prefix and suffix values and key off those.

I was thinking ahead a bit about an alternate implementation, and what contracts these implementations should share. Having the interface/abstract class was part of that thought process.

@nealeu
Copy link

nealeu commented Dec 20, 2012

Agree. Keeping consistent with the existing prefix and suffix approaches would be good.

Regarding abstract classes, I would say that the parent is AbstractDelegatingViewResolver which has an abstract protected method for assembling the view name, which is called from resolveViewName.

Also, do you need setViewResolver(delegate) given that it is required and is set on the constructor. I'd prefer it was immutable.

@royclarkson
Copy link
Contributor Author

Indeed, I've already dropped the interface and moved the getDeviceViewName to a protected abstract method. As you say, we don't need the setViewResolver method either, now that I'm removing the interface.

@royclarkson
Copy link
Contributor Author

I've made several improvements and adjustments based on discussions and feedback. I still need to polish the javadoc and update tests, but here's what I have so far.

@royclarkson
Copy link
Contributor Author

I've rebased in copyright date updates and moved the device and sitepreference code out of the abstract class.

@royclarkson royclarkson merged commit 0cdc4b4 into spring-attic:master Jan 14, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants