|

»

findbugs-discuss

isn't final but should be

View: New views

5 Messages — Rating Filter: Alert me

	isn't final but should be by gardner_art :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Some parts of this message have been removed. Learn more about Nabble's security policy. Hi! I got flagged for this: <sample> public abstract class XYZ { … protected static String theString = “whatever”; … static { String property = codeToLookUpProperty(); If (property != null) theString = property; } </sample> From the java doc (core java) I could not be sure whether “final” would be OK here, but I tried it, and java 1.5 flagged the last statement, setting theString, as an error. So, is this a case just not worth the trouble of checking for, or is there something wrong with my code? The idea is that the value is constant for each execution of the server containing this class, so I want to set it only once, when the class is loaded. Art Gardner _______________________________________________ Findbugs-discuss mailing list Findbugs-discuss@... https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss
	Re: isn't final but should be by Tim Halloran-2 :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Actually you can make the field final in this case. The key is to ensure that there is only a single assignment along any possible control path. public abstract class XYZ { protected final static String theString; static { final String property = codeToLookUpProperty(); if (property != null) theString = property; else theString = "whatever"; } On Fri, Aug 21, 2009 at 12:19 PM, <gardner_art@...> wrote: > > Hi! > > > > I got flagged for this: > > > > <sample> > > public abstract class XYZ { > > … > > protected static String theString = “whatever”; > > … > > static { > > String property = codeToLookUpProperty(); > > If (property != null) > > theString = property; > > } > > </sample> > > > > From the java doc (core java) I could not be sure whether “final” would be OK here, but I tried it, and java 1.5 flagged the last statement, setting theString, as an error. > > > > So, is this a case just not worth the trouble of checking for, or is there something wrong with my code? The idea is that the value is constant for each execution of the server containing this class, so I want to set it only once, when the class is loaded. > > > > Art Gardner > > _______________________________________________ > Findbugs-discuss mailing list > Findbugs-discuss@... > https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss > -- Tim Halloran SureLogic, Inc. 5808 Forbes Avenue, Pittsburgh PA 15217-1602 (412) 722-3338 _______________________________________________ Findbugs-discuss mailing list Findbugs-discuss@... https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss
	Re: isn't final but should be by Dave Brosius-2 :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Some parts of this message have been removed. Learn more about Nabble's security policy. there is a bug in the detector -----Original Message----- From: gardner_art@... Sent: Friday, August 21, 2009 12:19pm To: findbugs-discuss@... Subject: [FB-Discuss] isn't final but should be Hi! I got flagged for this: <sample> public abstract class XYZ { … protected static String theString = “whatever”; … static { String property = codeToLookUpProperty(); If (property != null) theString = property; } </sample> From the java doc (core java) I could not be sure whether “final” would be OK here, but I tried it, and java 1.5 flagged the last statement, setting theString, as an error. So, is this a case just not worth the trouble of checking for, or is there something wrong with my code? The idea is that the value is constant for each execution of the server containing this class, so I want to set it only once, when the class is loaded. Art Gardner _______________________________________________ Findbugs-discuss mailing list Findbugs-discuss@... https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss
	Re: isn't final but should be by gardner_art :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Some parts of this message have been removed. Learn more about Nabble's security policy. Well, I was satisfied with the previous response. There is, indeed, a simple way to make this variable final. Art From: Dave Brosius [mailto:dbrosius@...] Sent: Friday, August 21, 2009 11:39 AM To: Gardner, Art Cc: findbugs-discuss@... Subject: RE: [FB-Discuss] isn't final but should be there is a bug in the detector -----Original Message----- From: gardner_art@... Sent: Friday, August 21, 2009 12:19pm To: findbugs-discuss@... Subject: [FB-Discuss] isn't final but should be Hi! I got flagged for this: <sample> public abstract class XYZ { … protected static String theString = “whatever”; … static { String property = codeToLookUpProperty(); If (property != null) theString = property; } </sample> From the java doc (core java) I could not be sure whether “final” would be OK here, but I tried it, and java 1.5 flagged the last statement, setting theString, as an error. So, is this a case just not worth the trouble of checking for, or is there something wrong with my code? The idea is that the value is constant for each execution of the server containing this class, so I want to set it only once, when the class is loaded. Art Gardner _______________________________________________ Findbugs-discuss mailing list Findbugs-discuss@... https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss
	Re: isn't final but should be by Thomas Hawtin :: Rate this Message: Reply to Author \| View Threaded \| Show Only this Message Tim Halloran wrote: > Actually you can make the field final in this case. The key is to > ensure that there is only a single assignment along any possible > control path. There is a tediously large set of (actually intuitive) rules as to what constitutes a single assignment (Chapter 13(?) of the JLS 3rd Ed.). Note, that you are not allowed to qualify the assigned variable with the class name, which is often a pain. IIRC, Effective Java recommends putting the computational logic inside a static method and calling that in the initialisation expression. From a security perspective, anywhere that untusted code can diddle with a static is a potential problem, if only an annoyance (but may be much worse). See 6u15 release notes where a number of issues have been fixed. http://java.sun.com/javase/6/webnotes/6u15.html Mutable static are also a pain from testing and a dependency point of view. At the end of the day, they are bad design. Slightly off topic, "protected static" never makes sense even for constants and methods. Implementations should not be forced to be a big ball of mud subclass. Tom _______________________________________________ Findbugs-discuss mailing list Findbugs-discuss@... https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss