Re: Don't give me static...
by Justin Harris
----- "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
13 years, 11 months
Don't give me static...
by Justin Harris
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.
So I think that the lesson learned is: Don't use static anything. Or, 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.
What do you guys think? Am I overreacting? :)
- Justin
13 years, 11 months
Building Candlepin in Brew using Mead
by Adam Young
Mike Bonnet was good enough to spend a couple of hours with me getting
me up to speed on what he is doing with Mead. The short of it is that
he has a middle grouind between mavne repository fetching and JPackage
for building Maven based projects.
In order to take advantage of it, we have one of two choices.
1. Move Candlepin over to maven
2. Help him get Mead able to use buildr.
Note that this would produce a usable build without having to use all of
the MySpec RPMS I built. We could selectively use them if we desire.
I like option 2, although I might not be able to see it through. In
order to do this, we have to close the loop on buildr doing local
repository builds. As I can see it this means modifying the -r
localbuild option such that, once the localbuild script has modified the
remote and local repository values, they can no longer be modified by
the buildfile. I think I can make this happen, I'll let you know. It
might take an additional patch to buildr.
We also need to hack the emma code inside of buildr to not try and
download emma from a remote repository.
Once that is done, Mike needs to provide an extension to the command
line interface to brew: something like brew-buildr, and then modify how
that calls brew such that it knows to do a buildr -r localbuild type
build. The localbuild file will be something that he controls as part
of the mead/brew system, and that will manage the repos used to build
candlepin.
13 years, 11 months
