3:55 p.m.
----- "Jesus M. Rodriguez" <jesusr(a)redhat.com> wrote:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 06/01/2010 04:16 PM, Justin Harris wrote:
> I was just burned pretty bad by a subtle bug and want to share some
> general thoughts with folks. The core of the issue that I had was
that
> the Config class (which is not static) held a static map that
represents
> the internal state of the object. Additionally, this static map
was
> only initialized upon the first creation of a Config object. This
> doesn't really cause much harm in a deployed environment, but it
caused
> a lot of pain in our unit test suite -- specifically why one of the
> tests would pass when run by itself, and yet failed when run in the
> larger test suite.
But why is this the case? is it that we are changing the config in
one
test? If a test changes the config, they should change it BACK.
Why have to worry about all this state juggling, when each test can
start with a blank slate and create what it needs? I agree that there
are situations where that might not be as easy to do, or more expensive
than is tolerable, but this is not one of those cases.
> So I think that the lesson learned is: Don't use static anything.
Or,
I wouldn't go as far as saying "don't use static anything", I still
think
there are some uses for it. Like all code we just need to be careful
how we use things. Using static doesn't necessarily make the code bad
just like not using static doesn't make the code good.
I agree with this, but it does introduce unnecessary coupling between components,
one of the very practices that frameworks such as Guice are aiming to overcome. You make
a good point in the bootstrap process, as getting Guice up and running may require us
to make use of static classes, but I do think that as much as possible, we should inject
dependencies and let Guice handle tying them together.
> at the very least, treat static classes/variables as a code smell,
and
> really try to justify it before throwing it in. One of the really
nice
> aspects of Guice is its ability to abstract away how the
dependencies of
> a component are created, and I think we should defer to the
framework
> for e.g. creating objects as singletons for optimization purposes.
Since we are using Guice, it does make sense that we would use it to
create a singleton of the Config, but how does that affect the custom
modules we load in the Config to configure Guice? Is this a chicken
and the egg problem?
Quite possibly, maybe we are overloading the Config class too much? One possible
solution is to split the config into:
1) Config to get Guice up and running
2) Config component that can be injected into other objects
> What do you guys think? Am I overreacting? :)
See above, and static's aren't always bad.
- --
jesus m. rodriguez | jesusr(a)redhat.com
principal software engineer | irc: zeus
red hat systems management | 919.754.4413 (w)
rhce # 805008586930012 | 919.623.0080 (c)
+---------------------------------------------+
| "Those who cannot remember the past |
| are condemned to repeat it." |
| -- George Santayana |
+---------------------------------------------+
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkwFb6YACgkQvJZ57YntiYN3GwCfbvv1qQsj9wQv7LJgCC4STvhJ
/poAnip3xxAGjvbg09zzEwgZVYi/MlpH
=H/Uy
-----END PGP SIGNATURE-----
_______________________________________________
candlepin mailing list
candlepin(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/candlepin