Gilles Sadowski wrote: >> >> >> >> > URL: http://svn.apache.org/viewvc?rev=1245061&view=rev >> >> >> >> > Log: >> >> >> >> > Removed unneeded clone. >> >> >> >> > >> >> >> >> > The clone did not protect the array used, only the reference >> >> >> >> > ones. JIRA: MATH-650 >> >> >> >> >> >> >> >> -1 >> >> >> >> >> >> >> >> That was the whole point of the clone - to protect the original >> >> >> >> external data. >> >> >> > >> >> >> > Please (re-)explain what you mean by "protect". Cf. my comment >> >> >> > on the JIRA page. >> >> >> >> >> >> See also your comment of 30/Nov/11 00:31. >> >> > >> >> > I know, but my latest comment overrides that one. >> >> > >> >> >> The arrays in FastMathLiteralArrays are private, but the access >> >> >> methods are not, and returning an array allows the caller to modify >> >> >> array elements. >> >> > >> >> > It's true, but unless I'm mistaken, it doesn't matter since in the >> >> > end there is one and only one array that will be _used_ (the >> >> > modified one) and having a pristine copy somewhere else will not >> >> > prevent the dire consequences of using the modified one... ;-/ >> >> >> >> Not sure I follow that. >> >> >> >> Without clone, the array entries can be changed at any time. >> >> >> >> With clone, there are two pristine copies; neither can be changed >> >> directly as they are stored in private arrays. >> > >> > The question is: Why calling "clone"? >> > The answer is: You get a copy and you cannot change the array stored in >> > "FastMathLiteralArrays". >> > But that doesn't save you from the bug that consists in changing an >> > array entry from within "FastMath". >> > If that potential bug exists, some methods will produce erroneous >> > values because they use the copy, not the pristine original array >> > stored in "FastMathLiteralArrays". [After loading, the original array >> > is never accessed again.] >> > >> > Now if you just obtain a reference to the original array (i.e. no call >> > to "clone"), the bug will indeed modify it. >> > But the consequences are not worse than in the above scenario: The >> > exact same erroneous values will be computed. >> >> Not strictly true, because it's not only the FastMath class that can >> corrupt the array. > > Only "our" classes (in the package "o.a.c.m.util") can access (and > corrupt) the arrays. > >> >> Of course clone does not protect against bugs in FastMath; only array >> entry getters can do that. > > Yes. That's what I said. > >> But that's not the point; > > Yes, that's the point. > >> since the arrays were (at your insistence) >> moved out of FastMath itself, this necessarily meant exposing the >> contents to some degree. >> Using clone closes that potential exposure - as would using array entry >> getters. > > "clone" provides protection against CM developers, which I deem > unnecessary, since those same developers would also be able to make the > "mistake" of modifying the contents "FastMathLiteralArrays". [And the same > mistake could also happen if the arrays were stored inside the "FastMath" > class.]
I support Gilles here, the methods are package scoped and therefore internal. A CM user will have to create an own package with same name (which will not work for sealed jars or with OSGi bundles anyway) or use reflection to access them. At that stage he should already know what he does. For CM itself there's no need to pay the penalty to create a copy. - Jörg --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
