Thanks for the reply.
I've been trying to let Guice do as much object creation as possible.
Perhaps I've taken this too far...
So just to summarise what you have recommended:
1) Don't bother injecting empty collections, or other simple objects
that don't need to be unit tested.
2) Don't bother injecting objects whose sole purpose is to clone other
simple objects, just use one of the static Collections methods, or new
it up youself.
3) Unit test my helper classes to ensure that it does not return the
internal Map.
I suppose my problem is working out when it is appropriate for Guice
to create the object, and when it is appropriate for me to create the
object.
As you spell out in your blog, IOC is about making unit testing
easier, and as it is not necessary to unit test core java components
such as empty maps, it is not worth the effort to get Guice to create
them. However, I don't think I'd go as far to say injecting an empty
map was container abuse. For example, if I wanted to see what the
effects of using different Map implementations were, it would be nice
to have the empty map type defined in configuration to aid comparison
testing. Perhaps what to take from this is that you should only get
Guice to inject empty collections if you have a concrete for doing so.
Thanks,
Matt.
Eric Anderson wrote:
> Matt,
>
> Unless something outside of your MapHelper needs a reference to the same
> instance of the map, I would not pass the empty collection into the
> constructor. I would have the MapHelper create the map (via normal
> construction) in the constructor of the MapHelper. I might even just use a
> field initializer.
>
> I never use DI to create empty collections. I have used it to create
> populated collections (say, with a bunch of validation strategies).
>
> If you have concerns about returning the actual internal instance, simply
> write some unit tests that force the behavior. I would probably use static
> methods for actually cloning the collection. In this case, I would probably
> write some JUnit tests as follows:
>
> public class MapHelper_when_populated_Test {
>
> private MapHelper helper;
>
> @Before
> public void setup() {
> helper = new MapHelper();
> helper.doSomethingThatAddsToTheMap("key", "value");
> }
>
> @Test
> public void mapHelper_should_keep_the_things_that_are_added() {
> assertEquals("value", helper.getMap().get("key"));
> }
>
> @Test
> public void mapHelper_should_not_expose_its_internal_map() {
> Map map1 = helper.getMap();
> Map map2 = helper.getMap();
>
> //If the instances are different, then the helper is probably not
> exposing the internal collection
> assertFalse(map1 == map2);
>
> //Just to make sure nothing funny is going on
> map1.put("key2", "value2");
> assertNull(map2.get("key2"));
> }
> }
>
> The second bit on the second test is probably not needed. But, this ensures
> that you aren't getting back different wrapper instances that are backed by
> the same internal class (I've seen this behavior before).
>
> I started writing much more about when I actuall do provide collection from
> Guice, and it turned into a blog post:
>
> http://testinfected.blogspot.com/2009/01/when-collections-are-configuration.html
>
> Eric
>
> On Sat, Jan 31, 2009 at 1:03 PM, Matt G <[email protected]> wrote:
>
> >
> > Hi,
> >
> > Let me start by stating that I am very new to dependency injection and
> > Guice (1 week ish), so it is entirely possible that some, if not all
> > of the following questions will be rather silly... anyway, with the
> > disclaimer out the way, here I go...
> >
> > Let's say I have some sort of helper class, that populates a map in a
> > particular way, and then returns the map so that it is appropriate for
> > use in another class via a Map<K,V> getMap() method. The helper class
> > will clearly have a dependency on some sort of empty map, so it makes
> > sense to have a Map passed directly into the constructor. So far so
> > good...
> >
> >
> > Q1)
> >
> > Given that the map is expected to be empty, it clearly makes sense to
> > check for this fact in the constructor, and throw an
> > IllegalArgumentException if it is not the case. I was considering on
> > creating an @Empty annotation, so that Empty collections can be
> > differentiated between non empty ones. If there are too many
> > implementing classes however, I'd need to create new ones
> > @EmptyHashMap, @ EmptyEnumMap etc...
> > How do people generally pass in empty collections into classes? Often
> > classes have dependencies on empty collections that are then
> > populated, and used internally without exposing the Map via the class
> > api. In both of the above situations, how do people generally pass in
> > these empty Collections?
> >
> > MapHelper
> > {
> >
> > {
> >
> > Ok, going back to the Helper class example. We have a class that
> > returns some sort of appropriately populated map via getMap().
> > However, if we want the helper class to be immutatble, we cannot
> > return the internal map directly, so we will want to create a new Map
> > based on the internal one. Therefore, the class will now have another
> > dependency. We need some sort of method that creates a new Map from an
> > old one. In other words we need some sort of class that has some sort
> > of Map<K,V> create(Map<> map) method.
> >
> > Q2)
> >
> > a) Should this new class implement Provider<Map<>> ?
> > Or should I just create a MapFactory interface, and associated
> > HashMapFactory implementation or whatever?
> >
> > b) Alternatively I could create some sot of HashMapFactory class that
> > has a createEmptyMap method, a createPopulatedMap(Map...) method etc,
> > and just pass this all emcompasing Map factory class into the
> > constructor. in this case there would no longer be any need to pass in
> > the empty map directly.
> >
> > The b) approach seems to make sense to me, but wanted to check with
> > others first. We're getting near the end of this post now, I
> > promise...
> >
> > Ok, so now I have another class, that internally expects a map of the
> > form created by the helper class.
> >
> > Q3
> > a) How would I best pass this to my class? by passing the helper
> > directly, or the Map directly???
> >
> >
> > class NewClass
> > {
> >
> > private final Map<> map;
> >
> > NewClass(Map<> map)
> > {
> > // To make class immutable need to do this.map = new
> > HashMap<..>(map) or something,
> > // so I need a new dependency to carry this out, a
> > MapFactory or something.
> > }
> > }
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > When creating a new class I find I will often have a dependency on
> > some sort of empty collection, that is used internally, but not
> > exposed via the class api. So with the goal of completly removing the
> > need for the new operator in my code, I pass this directly into the
> > constructor. Great, job done. However
> >
> >
> >
> > >
> >
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"google-guice" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/google-guice?hl=en
-~----------~----~----~----~------~----~------~--~---
