On Sat, Jan 31, 2009 at 5:22 PM, Matt G <[email protected]> wrote:
>
> Thanks for the reply.
No problem. Glad I could help.
>
> I've been trying to let Guice do as much object creation as possible.
> Perhaps I've taken this too far...
It's a learning that comes through experience. Like with tool, it can be
overused, and we typically find those limits by pushing them.
>
>
> 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.
Dhanji has summarized this very well (in many less words than I used :) ).
>
>
> 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.
Or, if you were simply characterizing the behavior, you could just as easily
change the created type in the constructor call... The simplest thing that
might possibly work and all that...
> 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.
>
Yes.
Eric
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
-~----------~----~----~----~------~----~------~--~---
