What I'm hearing in this exchange is akin to: "I do this all the time, and I've never had a problem, so you shouldn't worry about doing it, either." That's a dangerous attitude to take with anything in general and with concurrency in particular.
Even if you could show that no current JVM for x86 will ever do something as seemingly bizarre as moving initialization actions of non-final fields across a constructor boundary, do you really want to make that claim for all JVMs for all architectures and all time? Existing JVMs are already capable of astonishingly unintuitive code transformations. You don't want to have to add the disclaimer, "This code is safe for all JVMs that don't do certain weird, though legal, things." Final fields of an object are initialized properly even if a reference to that object is passed to another thread via a data race (assuming the object reference isn't leaked from the constructor). While that's not the case for non-final fields at the moment, the language might change some day to add that guarantee. Until that happy day, unless you can say that your code is entirely free of data races, final fields have a subtle safety advantage over non-final fields. I'd like to think that my code is race-free, but I don't have time to do a full concurrency audit of every library that I use. There's a very real possibility that somewhere in those dozens of transitive dependencies someone has overlooked a potential data race. Using a final field means one less place where I have to reason about and maybe defend against that possibility. My advice: Use final fields when possible without contortions, even in otherwise mutable classes. Prefer constructor injection. Embrace that extra constructor argument and that extra line in the constructor body for each final field. Eat food, mostly plants, not too much. (And again, this has nothing to do with using the final keyword with variables.) --tim On Sun, Nov 24, 2013 at 10:56 PM, Cédric Beust ♔ <[email protected]> wrote: > On Sun, Nov 24, 2013 at 6:42 PM, Tim Boudreau <[email protected]> wrote: > >> On Sunday, November 24, 2013 7:10:15 PM UTC-5, Cédric Beust ♔ wrote: >>> >>> Have you actually managed to make that program fail? I haven't, even in >>> a very tight loop (which I had to add), and your code doesn't loop, it's >>> just a one time run. >>> >> >> I don't think that program is going to do it. >> >> Thinking about it further, I'd say that field injection *is* equivalent >> to constructor injection if the following things are true: >> - You are not giving up any validity checks you would do in the >> constructor >> - You are nearly-guaranteed that this object can *only* be instantiated >> by dependency injection - something like the package-private FooImpl >> example I gave above (where you'd have to look up the @ImplementedBy >> annotation, and do a bunch of setAccessible() stuff to actually instantiate >> it - i.e. *anybody* would figure out that what they're doing is a really >> bad idea before they finish writing the code to do it) >> > > I think that pretty much covers it and that's what I was trying to > demonstrate, but maybe, Tim (the other one) will prove me wrong by > fulfilling my challenge. > > The second item of your list is actually implied, to me anyway: any object > in my code base that is constructed by injection has a private constructor, > so this guarantees that pretty much only a DI framework can instantiate it. > There are a couple of exceptions to this rule, though: > > - Having to provide a no arg constructor for other reasons (e.g. JSON > serialization). Never happy when I come across these since, obviously, the > JSON framework has no idea that injection is expected for such objects, but > I have encountered a few legitimate cases. > - The class is being injected by inheritance (i.e. it's the superclass of > an injected class). In such a case, the call to super must be explicitly > made by the subclass, implying a non-private constructor (package protected > or protected, most of the time) and therefore, technically invokable by > regular code. > > I usually catch these fairly early (NPE's because fields expected to be > injected are not), which are easy to remedy > > Nonetheless I don't think I'll ever be able to look at code with fields >> that could be final and aren't, and not mentally give the author a few >> demerits for sloppiness. >> > > You're welcome to, but I'll challenge you to prove that this code is > incorrect :) > > >> There might be a case for final fields in terms of what HotSpot will do >> with them, but much evil code has been written in the name of optimizing >> for whatever that day's version of the VM did with Java code (consider the >> mess in javax.text all built around the assumption that any object >> allocation is expensive; today that guarantees old objects, cache misses >> and more full GCs). So, tempting though it is, I don't think contorting >> one's code around assumptions about how the VM does things is ever a good >> idea. >> > > Definitely. > > -- > Cédric > > -- > You received this message because you are subscribed to the Google Groups > "google-guice" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/google-guice. > For more options, visit https://groups.google.com/groups/opt_out. > -- You received this message because you are subscribed to the Google Groups "google-guice" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/google-guice. For more options, visit https://groups.google.com/groups/opt_out.
