Re: [Math] About NullArgumentException
Hi, 2012/9/18 Phil Steitz phil.ste...@gmail.com: On 9/18/12 12:02 PM, Sébastien Brisard wrote: Hello, 2012/9/17 Gilles Sadowski gil...@harfang.homelinux.org: On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote: OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Thanks, Phil; we are making progress, and I hope that you'll be convinced of that at some point. Another decision still needs to be made. I think that everybody now agrees that wherever an argument will be directly used (i.e. dereferenced) inside the body of the method[1], it is safe to not check for null (since the JVM will throw an NPE). But, whenever an argument is passed for _later_ use (e.g. stored by a constructor or passed to another method), we also all expressed that it is better to fail early if the object is supposed to be non-null. In those cases, checks are not mandatory (since the JVM will throw NPE at some point), but we must agree on how optional checks are to be implemented. 1. The current state of affairs was to throw NullArgumentException; in 4.0 this exception will become a subclass of NPE and we can continue to use it in the same way as now (i.e. possibly providing additional localizable error messages). 2. The alternative is to directly throw a standard NPE, but without any message, since it wouldn't be localizable with the support of CM's context. So, which of those alternatives do people want to settle on? Regards, Gilles [1] As opposed to being passed on to another method. I have another question: are we going to little by little remove all null checks that we feel are not absolutely necessary? I would not mind going through this cleaning phase while working on MATH-854. I think we should wait to remove the existing null checks until 4.0. This is a significant behavior change, especially for the ones that have been in place and documented since 1.x. Removing the checks and allowing NPEs to propagate will break apps that catch IAE. Phil OK. I would like to say that I think this is going the right way: it is better to have no null checks than incomplete null checks which provide a false sense of security. For example, have a look to Array2DRowRealMatrix /** * Create a new RealMatrix using the input array as the underlying * data array. * If an array is built specially in order to be embedded in a * RealMatrix and not used directly, the {@code copyArray} may be * set to {@code false}. This will prevent the copying and improve * performance as no new array will be built and no data will be copied. * * @param d Data for new matrix. * @param copyArray if {@code true}, the input array will be copied, * otherwise it will be referenced. * @throws DimensionMismatchException if {@code d} is not rectangular * (not all rows have the same length) or empty. * @throws NullArgumentException if {@code d} is {@code null}. * @throws NoDataException if there are not at least one row and one column. * @see #Array2DRowRealMatrix(double[][]) */ public Array2DRowRealMatrix(final double[][] d, final boolean copyArray) { if (copyArray) { copyIn(d); } else { if (d == null) { throw new NullArgumentException(); } final int nRows = d.length; if (nRows == 0) { throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_ROW); } final int nCols = d[0].length; if (nCols == 0) { throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_COLUMN); } for (int r = 1; r nRows; r++) { if (d[r].length != nCols) { throw new DimensionMismatchException(d[r].length, nCols); } } data = d; } } I think if you want to be really helpful, a null check should also be carried out on each d[i]. I think I will leave it as is for now, but in 4.0, we should certainly remove this null check. Do you agree? Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/19/12 12:12 AM, Sébastien Brisard wrote: Hi, 2012/9/18 Phil Steitz phil.ste...@gmail.com: On 9/18/12 12:02 PM, Sébastien Brisard wrote: Hello, 2012/9/17 Gilles Sadowski gil...@harfang.homelinux.org: On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote: OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Thanks, Phil; we are making progress, and I hope that you'll be convinced of that at some point. Another decision still needs to be made. I think that everybody now agrees that wherever an argument will be directly used (i.e. dereferenced) inside the body of the method[1], it is safe to not check for null (since the JVM will throw an NPE). But, whenever an argument is passed for _later_ use (e.g. stored by a constructor or passed to another method), we also all expressed that it is better to fail early if the object is supposed to be non-null. In those cases, checks are not mandatory (since the JVM will throw NPE at some point), but we must agree on how optional checks are to be implemented. 1. The current state of affairs was to throw NullArgumentException; in 4.0 this exception will become a subclass of NPE and we can continue to use it in the same way as now (i.e. possibly providing additional localizable error messages). 2. The alternative is to directly throw a standard NPE, but without any message, since it wouldn't be localizable with the support of CM's context. So, which of those alternatives do people want to settle on? Regards, Gilles [1] As opposed to being passed on to another method. I have another question: are we going to little by little remove all null checks that we feel are not absolutely necessary? I would not mind going through this cleaning phase while working on MATH-854. I think we should wait to remove the existing null checks until 4.0. This is a significant behavior change, especially for the ones that have been in place and documented since 1.x. Removing the checks and allowing NPEs to propagate will break apps that catch IAE. Phil OK. I would like to say that I think this is going the right way: it is better to have no null checks than incomplete null checks which provide a false sense of security. For example, have a look to Array2DRowRealMatrix /** * Create a new RealMatrix using the input array as the underlying * data array. * If an array is built specially in order to be embedded in a * RealMatrix and not used directly, the {@code copyArray} may be * set to {@code false}. This will prevent the copying and improve * performance as no new array will be built and no data will be copied. * * @param d Data for new matrix. * @param copyArray if {@code true}, the input array will be copied, * otherwise it will be referenced. * @throws DimensionMismatchException if {@code d} is not rectangular * (not all rows have the same length) or empty. * @throws NullArgumentException if {@code d} is {@code null}. * @throws NoDataException if there are not at least one row and one column. * @see #Array2DRowRealMatrix(double[][]) */ public Array2DRowRealMatrix(final double[][] d, final boolean copyArray) { if (copyArray) { copyIn(d); } else { if (d == null) { throw new NullArgumentException(); } final int nRows = d.length; if (nRows == 0) { throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_ROW); } final int nCols = d[0].length; if (nCols == 0) { throw new NoDataException(LocalizedFormats.AT_LEAST_ONE_COLUMN); } for (int r = 1; r nRows; r++) { if (d[r].length != nCols) { throw new DimensionMismatchException(d[r].length, nCols); } } data = d; } } I think if you want to be really helpful, a null check should also be carried out on each d[i]. I think I will leave it as is for now, but in 4.0, we should certainly remove this null check. Do you agree? Yes, the API should be nullsafe or not. The half-way solution above is no good and it makes the javadoc misleading. I would be OK actually adding the check at the row level now (or catching the NPE and throwing DME in its place) so what the javadoc says is true (d null means NAE and d non-rectangular or empty means DME). On the other hand, this is unlikely to ever happen in practice, so I would leave as is for now. Phil Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands,
Re: [Math] About NullArgumentException
Hello, 2012/9/17 Gilles Sadowski gil...@harfang.homelinux.org: On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote: OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Thanks, Phil; we are making progress, and I hope that you'll be convinced of that at some point. Another decision still needs to be made. I think that everybody now agrees that wherever an argument will be directly used (i.e. dereferenced) inside the body of the method[1], it is safe to not check for null (since the JVM will throw an NPE). But, whenever an argument is passed for _later_ use (e.g. stored by a constructor or passed to another method), we also all expressed that it is better to fail early if the object is supposed to be non-null. In those cases, checks are not mandatory (since the JVM will throw NPE at some point), but we must agree on how optional checks are to be implemented. 1. The current state of affairs was to throw NullArgumentException; in 4.0 this exception will become a subclass of NPE and we can continue to use it in the same way as now (i.e. possibly providing additional localizable error messages). 2. The alternative is to directly throw a standard NPE, but without any message, since it wouldn't be localizable with the support of CM's context. So, which of those alternatives do people want to settle on? Regards, Gilles [1] As opposed to being passed on to another method. I have another question: are we going to little by little remove all null checks that we feel are not absolutely necessary? I would not mind going through this cleaning phase while working on MATH-854. Best regards, Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/18/12 12:02 PM, Sébastien Brisard wrote: Hello, 2012/9/17 Gilles Sadowski gil...@harfang.homelinux.org: On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote: OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Thanks, Phil; we are making progress, and I hope that you'll be convinced of that at some point. Another decision still needs to be made. I think that everybody now agrees that wherever an argument will be directly used (i.e. dereferenced) inside the body of the method[1], it is safe to not check for null (since the JVM will throw an NPE). But, whenever an argument is passed for _later_ use (e.g. stored by a constructor or passed to another method), we also all expressed that it is better to fail early if the object is supposed to be non-null. In those cases, checks are not mandatory (since the JVM will throw NPE at some point), but we must agree on how optional checks are to be implemented. 1. The current state of affairs was to throw NullArgumentException; in 4.0 this exception will become a subclass of NPE and we can continue to use it in the same way as now (i.e. possibly providing additional localizable error messages). 2. The alternative is to directly throw a standard NPE, but without any message, since it wouldn't be localizable with the support of CM's context. So, which of those alternatives do people want to settle on? Regards, Gilles [1] As opposed to being passed on to another method. I have another question: are we going to little by little remove all null checks that we feel are not absolutely necessary? I would not mind going through this cleaning phase while working on MATH-854. I think we should wait to remove the existing null checks until 4.0. This is a significant behavior change, especially for the ones that have been in place and documented since 1.x. Removing the checks and allowing NPEs to propagate will break apps that catch IAE. Phil Best regards, Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote: OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Thanks, Phil; we are making progress, and I hope that you'll be convinced of that at some point. Another decision still needs to be made. I think that everybody now agrees that wherever an argument will be directly used (i.e. dereferenced) inside the body of the method[1], it is safe to not check for null (since the JVM will throw an NPE). But, whenever an argument is passed for _later_ use (e.g. stored by a constructor or passed to another method), we also all expressed that it is better to fail early if the object is supposed to be non-null. In those cases, checks are not mandatory (since the JVM will throw NPE at some point), but we must agree on how optional checks are to be implemented. 1. The current state of affairs was to throw NullArgumentException; in 4.0 this exception will become a subclass of NPE and we can continue to use it in the same way as now (i.e. possibly providing additional localizable error messages). 2. The alternative is to directly throw a standard NPE, but without any message, since it wouldn't be localizable with the support of CM's context. So, which of those alternatives do people want to settle on? Regards, Gilles [1] As opposed to being passed on to another method. Phil On 9/14/12 8:46 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote: On 9/14/12 4:28 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Why is it better to throw MathIllegalArgumentException rather than NPE? Because, as I have repeated numerous times, that is the appropriate exception to throw when API preconditions are violated. That is why IAE exists. The class javadoc comment for IAE is nice and succinct (we could learn from it ;) IllegalArgumentException: --- Thrown to indicate that a method has been passed an illegal or inappropriate argument. --- Did you read the class Javadoc for NPE? Here is the last sentence pasted again: --- Applications should throw instances of this class to indicate other illegal uses of the null object. --- You could ask the same question about ArrayIndexOutOfBounds. Why not throw that instead of IAE when bad indices are provided? Does CM check _array_ index? No. Surprised? An ArrayRealVector is _not_ an array, it is the representation of the vector concept. That it is a backed by a Java array is an implemtation detail that should not surface to the API. Hence the choice to not use the low-level ArrayIndexOutOfBoundsException can be given a rationale. CM checks that you access valid entries of the high-level mathematical object. And we do that for _all_ such accesses and never (or it is a CM bug) let propagate an exception that reveals an underlying Java array. [This was one of my proposals
Re: [Math] About NullArgumentException
Le 17/09/2012 11:50, Gilles Sadowski a écrit : On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote: OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Thanks, Phil; we are making progress, and I hope that you'll be convinced of that at some point. Another decision still needs to be made. I think that everybody now agrees that wherever an argument will be directly used (i.e. dereferenced) inside the body of the method[1], it is safe to not check for null (since the JVM will throw an NPE). But, whenever an argument is passed for _later_ use (e.g. stored by a constructor or passed to another method), we also all expressed that it is better to fail early if the object is supposed to be non-null. In those cases, checks are not mandatory (since the JVM will throw NPE at some point), but we must agree on how optional checks are to be implemented. 1. The current state of affairs was to throw NullArgumentException; in 4.0 this exception will become a subclass of NPE and we can continue to use it in the same way as now (i.e. possibly providing additional localizable error messages). 2. The alternative is to directly throw a standard NPE, but without any message, since it wouldn't be localizable with the support of CM's context. So, which of those alternatives do people want to settle on? I would prefer 1. If we fail early, do it as precisely as possible. Luc Regards, Gilles [1] As opposed to being passed on to another method. Phil On 9/14/12 8:46 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote: On 9/14/12 4:28 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Why is it better to throw MathIllegalArgumentException rather than NPE? Because, as I have repeated numerous times, that is the appropriate exception to throw when API preconditions are violated. That is why IAE exists. The class javadoc comment for IAE is nice and succinct (we could learn from it ;) IllegalArgumentException: --- Thrown to indicate that a method has been passed an illegal or inappropriate argument. --- Did you read the class Javadoc for NPE? Here is the last sentence pasted again: --- Applications should throw instances of this class to indicate other illegal uses of the null object. --- You could ask the same question about ArrayIndexOutOfBounds. Why not throw that instead of IAE when bad indices are provided? Does CM check _array_ index? No. Surprised? An ArrayRealVector is _not_ an array, it is the representation of the vector concept. That it is a backed by a Java array is an implemtation detail that should not surface to the API. Hence the choice to not use the low-level ArrayIndexOutOfBoundsException can be given a rationale. CM checks that you access valid entries of the high-level mathematical object. And we do that for _all_ such accesses and never (or it is a CM bug) let propagate an exception
Re: [Math] About NullArgumentException
Hi Gilles, 2012/9/17 Luc Maisonobe luc.maison...@free.fr: Le 17/09/2012 11:50, Gilles Sadowski a écrit : On Fri, Sep 14, 2012 at 11:29:41AM -0700, Phil Steitz wrote: OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Thanks, Phil; we are making progress, and I hope that you'll be convinced of that at some point. Another decision still needs to be made. I think that everybody now agrees that wherever an argument will be directly used (i.e. dereferenced) inside the body of the method[1], it is safe to not check for null (since the JVM will throw an NPE). But, whenever an argument is passed for _later_ use (e.g. stored by a constructor or passed to another method), we also all expressed that it is better to fail early if the object is supposed to be non-null. In those cases, checks are not mandatory (since the JVM will throw NPE at some point), but we must agree on how optional checks are to be implemented. 1. The current state of affairs was to throw NullArgumentException; in 4.0 this exception will become a subclass of NPE and we can continue to use it in the same way as now (i.e. possibly providing additional localizable error messages). 2. The alternative is to directly throw a standard NPE, but without any message, since it wouldn't be localizable with the support of CM's context. So, which of those alternatives do people want to settle on? I would prefer 1. If we fail early, do it as precisely as possible. Luc Regards, Gilles [1] As opposed to being passed on to another method. I would also be in favor of #1, so that any exception explicitely thrown by CM is one of our own exceptions. Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Best regards, Sébastien Your alternative (sometimes check, sometimes not) is not fully consistent: * for the user: In case of null reference, will I get a MathRuntimeException or a NPE? * for the CM developer: In which cases do I need to check for null? Of course, I would reconsider if you could provide an actual example that would fail with all three alternatives which I suggested. If not, then it seems obvious that your alternative presents two defects that don't exist in any of those three. Gilles [...] what is different about null arguments at the point of method activation in an API that explicitly disallows them. The difference is that there is no need to tell the user what the problem is because the raised exception says it all: You tried to use a null reference. [As said above, the only issue is _when_ the exception is triggered.] The policy mandates what is globally valid, e.g.: If not specified otherwise, null is not allowed as an argument. Then, a user cannot complain about a NPE being propagated when he passed an invalid (null) argument. Finally, the case for null is also slightly peculiar (given the above) that it should not simply be bundled with the rationale Fail early: Indeed null references always fail early (i.e. as soon as they are used). Deferring the check until it is done by the JVM will never entails the code to produce a wrong result _because_ of that null reference (it will just fail in the predictable way: NPE).[1] Gilles [1] Unlike in C, where an unitialized pointer would not necessarily crash the program, but _could_ make it behave erratically (including produce wrong results in a stealth way). - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. My preferences are 4 or 3 with same preference level and 2 as a fallback choice. For each option, I find the arguments are good, it's only a matter or preference. Luc Best regards, Sébastien Your alternative (sometimes check, sometimes not) is not fully consistent: * for the user: In case of null reference, will I get a MathRuntimeException or a NPE? * for the CM developer: In which cases do I need to check for null? Of course, I would reconsider if you could provide an actual example that would fail with all three alternatives which I suggested. If not, then it seems obvious that your alternative presents two defects that don't exist in any of those three. Gilles [...] what is different about null arguments at the point of method activation in an API that explicitly disallows them. The difference is that there is no need to tell the user what the problem is because the raised exception says it all: You tried to use a null reference. [As said above, the only issue is _when_ the exception is triggered.] The policy mandates what is globally valid, e.g.: If not specified otherwise, null is not allowed as an argument. Then, a user cannot complain about a NPE being propagated when he passed an invalid (null) argument. Finally, the case for null is also slightly peculiar (given the above) that it should not simply be bundled with the rationale Fail early: Indeed null references always fail early (i.e. as soon as they are used). Deferring the check until it is done by the JVM will never entails the code to produce a wrong result _because_ of that null reference (it will just fail in the predictable way: NPE).[1] Gilles [1] Unlike in C, where an unitialized pointer would not necessarily crash the program, but _could_ make it behave erratically (including produce wrong results in a stealth way). - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Why is it better to throw MathIllegalArgumentException rather than NPE? My preferences are 4 or 3 with same preference level and 2 as a fallback choice. For each option, I find the arguments are good, it's only a matter or preference. Did everyone consider that Phil's alternative implies a behaviour that departs from the standard practice, a.o. * JDK (cf. quote in a previous post), * other libraries, e.g. Guava (cf. quote in a previous post), and * Java experts, e.g. Joshua Bloch[1] of throwing NPE when they encounter null? Doesn't anyone care about having a _reason_ for CM to behave differently? Don't you care that users will see different exceptions when the same problem is detected? Do you really like a policy that mandates different behaviours when null is disallowed implicitly vs explicitly (throwing NPE vs throwing MathRuntimeException)? [Pardon me, but IMHO this is nonsense.] After piling up arguments (_none_ of which have been addressed), I'm sorry to read that it would only be a matter of preference! [I should start a career as a writer if I'm able to express preference in so many words...] Actually, it's a matter of consistency (same problem, same exception). If nobody cares about improving CM's consistency, then indeed most of the discussions in which I take part are pointless. What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows CM to fail as early as possible (which I thought was Phil's main point, as it was his only one, apart from preference), while being consistent (same problem, same exception), internally, externally, implicitly and explicitly. Alternative 4 is inconsistent (same problem, different exceptions); it's a truth, not a matter of taste.] Rather then being willingly stuck in a deadlock because only the weakest argument (preference) is taken into account, wouldn't it be more reasonable to count all the arguments? Gilles [1] Effective Java (second edition). Excerpt from Item 60: If a caller passes null in some parameter for which null values are prohibited, convention dictates that NullPointerException be thrown rather than IllegalArgumentException. Luc Best regards, Sébastien Your alternative (sometimes check, sometimes not) is not fully consistent: * for the user: In case of null reference, will I get a MathRuntimeException or a NPE? * for the CM developer: In which cases do I need to check for null? Of course, I would reconsider if you could provide an actual example that would fail with all three alternatives which I suggested. If not, then it seems obvious that your alternative presents two defects that don't exist in any of those three. Gilles [...] what is different about null arguments at the point of method activation in an API that explicitly disallows them. The difference is that there is no need to tell the user what the problem is because the raised exception says it all: You tried to use a null reference. [As said above, the only issue is _when_ the
Re: [Math] About NullArgumentException
Hi Gilles, 2012/9/14 Gilles Sadowski gil...@harfang.homelinux.org: On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Why is it better to throw MathIllegalArgumentException rather than NPE? Sorry, I misread Phil's proposal: I like the fact that nulls are checked only if stated in the doc. But I also think that the exception thrown should inherit from NPE, not IAE. Your previous messages convinced me, but you have to admit that you all wrote a lot on this issue, so it's difficult to keep up. Nevertheless, I should have read more carefully. Sébastien My preferences are 4 or 3 with same preference level and 2 as a fallback choice. For each option, I find the arguments are good, it's only a matter or preference. Did everyone consider that Phil's alternative implies a behaviour that departs from the standard practice, a.o. * JDK (cf. quote in a previous post), * other libraries, e.g. Guava (cf. quote in a previous post), and * Java experts, e.g. Joshua Bloch[1] of throwing NPE when they encounter null? Doesn't anyone care about having a _reason_ for CM to behave differently? Don't you care that users will see different exceptions when the same problem is detected? Do you really like a policy that mandates different behaviours when null is disallowed implicitly vs explicitly (throwing NPE vs throwing MathRuntimeException)? [Pardon me, but IMHO this is nonsense.] After piling up arguments (_none_ of which have been addressed), I'm sorry to read that it would only be a matter of preference! [I should start a career as a writer if I'm able to express preference in so many words...] Actually, it's a matter of consistency (same problem, same exception). If nobody cares about improving CM's consistency, then indeed most of the discussions in which I take part are pointless. What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows CM to fail as early as possible (which I thought was Phil's main point, as it was his only one, apart from preference), while being consistent (same problem, same exception), internally, externally, implicitly and explicitly. Alternative 4 is inconsistent (same problem, different exceptions); it's a truth, not a matter of taste.] Rather then being willingly stuck in a deadlock because only the weakest argument (preference) is taken into account, wouldn't it be more reasonable to count all the arguments? Gilles [1] Effective Java (second edition). Excerpt from Item 60: If a caller passes null in some parameter for which null values are prohibited, convention dictates that NullPointerException be thrown rather than IllegalArgumentException. Luc Best regards, Sébastien Your alternative (sometimes check, sometimes not) is not fully consistent: * for the user: In case of null reference, will I get a MathRuntimeException or a NPE? * for the CM developer: In which cases do I need to check for null? Of course, I would reconsider if you could provide an actual example that would fail with all three alternatives which I suggested. If
Re: [Math] About NullArgumentException
On 9/14/12 4:28 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Why is it better to throw MathIllegalArgumentException rather than NPE? Because, as I have repeated numerous times, that is the appropriate exception to throw when API preconditions are violated. That is why IAE exists. The class javadoc comment for IAE is nice and succinct (we could learn from it ;) You could ask the same question about ArrayIndexOutOfBounds. Why not throw that instead of IAE when bad indices are provided? The difference is that instead of letting the precondition failure go undetected and the runtime exception propagate, APIs with explicit preconditions and parameter checking in place raise the exception at method activation time. In that context, what is appropriate is IAE. Phil My preferences are 4 or 3 with same preference level and 2 as a fallback choice. For each option, I find the arguments are good, it's only a matter or preference. Did everyone consider that Phil's alternative implies a behaviour that departs from the standard practice, a.o. * JDK (cf. quote in a previous post), * other libraries, e.g. Guava (cf. quote in a previous post), and * Java experts, e.g. Joshua Bloch[1] of throwing NPE when they encounter null? Doesn't anyone care about having a _reason_ for CM to behave differently? Don't you care that users will see different exceptions when the same problem is detected? Do you really like a policy that mandates different behaviours when null is disallowed implicitly vs explicitly (throwing NPE vs throwing MathRuntimeException)? [Pardon me, but IMHO this is nonsense.] After piling up arguments (_none_ of which have been addressed), I'm sorry to read that it would only be a matter of preference! [I should start a career as a writer if I'm able to express preference in so many words...] Actually, it's a matter of consistency (same problem, same exception). If nobody cares about improving CM's consistency, then indeed most of the discussions in which I take part are pointless. What is wrong with throwing NPE? [Alternative 3 is a compromise: it allows CM to fail as early as possible (which I thought was Phil's main point, as it was his only one, apart from preference), while being consistent (same problem, same exception), internally, externally, implicitly and explicitly. Alternative 4 is inconsistent (same problem, different exceptions); it's a truth, not a matter of taste.] Rather then being willingly stuck in a deadlock because only the weakest argument (preference) is taken into account, wouldn't it be more reasonable to count all the arguments? Gilles [1] Effective Java (second edition). Excerpt from Item 60: If a caller passes null in some parameter for which null values are prohibited, convention dictates that NullPointerException be thrown rather than IllegalArgumentException. Luc Best regards, Sébastien Your alternative (sometimes check, sometimes not) is not fully consistent: * for the user: In case of null reference, will I get a MathRuntimeException or a NPE? * for the CM developer:
Re: [Math] About NullArgumentException
On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote: On 9/14/12 4:28 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Why is it better to throw MathIllegalArgumentException rather than NPE? Because, as I have repeated numerous times, that is the appropriate exception to throw when API preconditions are violated. That is why IAE exists. The class javadoc comment for IAE is nice and succinct (we could learn from it ;) IllegalArgumentException: --- Thrown to indicate that a method has been passed an illegal or inappropriate argument. --- Did you read the class Javadoc for NPE? Here is the last sentence pasted again: --- Applications should throw instances of this class to indicate other illegal uses of the null object. --- You could ask the same question about ArrayIndexOutOfBounds. Why not throw that instead of IAE when bad indices are provided? Does CM check _array_ index? No. Surprised? An ArrayRealVector is _not_ an array, it is the representation of the vector concept. That it is a backed by a Java array is an implemtation detail that should not surface to the API. Hence the choice to not use the low-level ArrayIndexOutOfBoundsException can be given a rationale. CM checks that you access valid entries of the high-level mathematical object. And we do that for _all_ such accesses and never (or it is a CM bug) let propagate an exception that reveals an underlying Java array. [This was one of my proposals too (alternative 1). However it imposes a unnecessary burden of duplicating (CM and JVM) every null check just for the sake of hiding NPE.] As indicated in a previous post, null is not a RealVector, it is not an Object, it is not a reference, it is just the value of an unitialized pointer. The difference is that instead of letting the precondition failure go undetected and the runtime exception propagate, APIs with explicit preconditions and parameter checking in place raise the exception at method activation time. Could you please stop mentioning this, since my proposal (alternative 3) allows for early detection? The difference is that you want to use MathIAE, where everyone else uses NPE. In that context, what is appropriate is IAE. You are stuck with a name, as if everything that is illegal must be signalled with an exception that contains the string Illegal in its name. If that is so, I propose to rename NullArgumentException to IllegalNullArgumentException. Gilles Phil My preferences are 4 or 3 with same preference level and 2 as a fallback choice. For each option, I find the arguments are good, it's only a matter or preference. Did everyone consider that Phil's alternative implies a behaviour that departs from the standard practice, a.o. * JDK (cf. quote in a previous post), * other libraries, e.g. Guava (cf. quote in a previous post), and * Java experts, e.g. Joshua Bloch[1] of throwing NPE when they encounter null? Doesn't anyone care about having a _reason_ for CM to behave differently? Don't you care that users will see different exceptions
Re: [Math] About NullArgumentException
OK, I give up. Lets do option 2. Just warn users in the User Guide somewhere that our APIs are in general not null-safe and unless the javadoc explicitly allows nulls, they can expect NPE when passing nulls. Phil On 9/14/12 8:46 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 07:44:08AM -0700, Phil Steitz wrote: On 9/14/12 4:28 AM, Gilles Sadowski wrote: On Fri, Sep 14, 2012 at 10:41:50AM +0200, Luc Maisonobe wrote: Le 14/09/2012 08:46, Sébastien Brisard a écrit : Hi Phil Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. As stated above, my preference is 4. CM APIs *may* disallow nulls explicitly. When that is done, the implementations *should* check parameters (as they *should* check all other stated preconditions) and when preconditions are violated, a MathIllegalArgumentException is thrown. I don't care whether or not we keep NAE. If we drop it, we should make sure whatever exception messages we used to use to indicate illegal null arguments are still there. Phil I like this option better than 3. So, I'll change my vote to option #2, and possibly option #4 as we will never agree on option #2. Why is it better to throw MathIllegalArgumentException rather than NPE? Because, as I have repeated numerous times, that is the appropriate exception to throw when API preconditions are violated. That is why IAE exists. The class javadoc comment for IAE is nice and succinct (we could learn from it ;) IllegalArgumentException: --- Thrown to indicate that a method has been passed an illegal or inappropriate argument. --- Did you read the class Javadoc for NPE? Here is the last sentence pasted again: --- Applications should throw instances of this class to indicate other illegal uses of the null object. --- You could ask the same question about ArrayIndexOutOfBounds. Why not throw that instead of IAE when bad indices are provided? Does CM check _array_ index? No. Surprised? An ArrayRealVector is _not_ an array, it is the representation of the vector concept. That it is a backed by a Java array is an implemtation detail that should not surface to the API. Hence the choice to not use the low-level ArrayIndexOutOfBoundsException can be given a rationale. CM checks that you access valid entries of the high-level mathematical object. And we do that for _all_ such accesses and never (or it is a CM bug) let propagate an exception that reveals an underlying Java array. [This was one of my proposals too (alternative 1). However it imposes a unnecessary burden of duplicating (CM and JVM) every null check just for the sake of hiding NPE.] As indicated in a previous post, null is not a RealVector, it is not an Object, it is not a reference, it is just the value of an unitialized pointer. The difference is that instead of letting the precondition failure go undetected and the runtime exception propagate, APIs with explicit preconditions and parameter checking in place raise the exception at method activation time. Could you please stop mentioning this, since my proposal (alternative 3) allows for early detection? The difference is that you want to use MathIAE, where everyone else uses NPE. In that context, what is appropriate is IAE. You are stuck with a name, as if everything that is illegal must be signalled with an exception that contains the string Illegal in its name. If that is so, I propose to rename NullArgumentException to IllegalNullArgumentException. Gilles Phil My preferences are 4 or 3 with same preference level and 2 as a fallback choice. For each option, I find the arguments are good, it's only a matter or preference. Did everyone consider that Phil's alternative implies a behaviour that departs from the standard practice, a.o. * JDK (cf. quote in a previous post), * other libraries, e.g. Guava (cf. quote in a previous
Re: [Math] About NullArgumentException
On Thu, Sep 13, 2012 at 01:10:41AM +0200, Gilles Sadowski wrote: Hello. [For those who don't wish to read the whole post, please at least go towards the end and indicate your preferred option. Thanks.] [... long post skipped ...] Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. My preference would be alternative 2 but I'd be happy with alternative 3 too (as a compromise to allow checks for people who need more time to get rid of the habit :-). With those additional arguments, for good measure: (a) --- Thrown when an application attempts to use null in a case where an object is required. These include: Calling the instance method of a null object. Accessing or modifying the field of a null object. Taking the length of null as if it were an array. Accessing or modifying the slots of null as if it were an array. Throwing null as if it were a Throwable value. Applications should throw instances of this class to indicate other illegal uses of the null object. --- [This is the Javadoc for NPE. Please note the last sentence.] (b) null is not a representation of an application-domain concept, it is a low low low level literal (the unique element of the null-type) and exists solely to be the value of unitialized pointers (not even reference). It is only perfectly right that attempts to use null are set apart from other bugs (that acquire meaning only wrt to higher level preconditions). (c) http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/base/Preconditions.html#checkNotNull(T) Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hi, 2012/9/13 Gilles Sadowski gil...@harfang.homelinux.org: On Thu, Sep 13, 2012 at 01:10:41AM +0200, Gilles Sadowski wrote: Hello. [For those who don't wish to read the whole post, please at least go towards the end and indicate your preferred option. Thanks.] [... long post skipped ...] Back to square one, with 3 fully consistent alternatives: 1. CM to always check for null? Then NullArgumentException inheriting from MathIllegalArgumentException is fine because we promise to the user that no NPE will ever propagate through the CM layer. [Breaking that promise is a CM bug.] 2. CM to never check for null? Then we delete class NullArgumentException. Users are warned by the general policy: Do not pass null unless it is explicitly documented to be allowed. A bug will lead to the JVM raising a NPE. 3. CM to sometimes check for null? Then NullArgumentException should inherit from NullPointerException because the user will sometimes see NullArgumentException (when CM checks) and sometimes NPE (when CM does not check) and both should thus belong to the same hierarchy (from the user's point-of-view, there is no reason to handle them in different ways since it's the exact same bug). For the user, the consequence will be similar to alternative 2, with sometimes more information about the failure and sometimes (marginally) earlier detection. My preference would be alternative 2 but I'd be happy with alternative 3 too (as a compromise to allow checks for people who need more time to get rid of the habit :-). I would also favor option #2. I can live with #3. Sébastien With those additional arguments, for good measure: (a) --- Thrown when an application attempts to use null in a case where an object is required. These include: Calling the instance method of a null object. Accessing or modifying the field of a null object. Taking the length of null as if it were an array. Accessing or modifying the slots of null as if it were an array. Throwing null as if it were a Throwable value. Applications should throw instances of this class to indicate other illegal uses of the null object. --- [This is the Javadoc for NPE. Please note the last sentence.] (b) null is not a representation of an application-domain concept, it is a low low low level literal (the unique element of the null-type) and exists solely to be the value of unitialized pointers (not even reference). It is only perfectly right that attempts to use null are set apart from other bugs (that acquire meaning only wrt to higher level preconditions). (c) http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/base/Preconditions.html#checkNotNull(T) Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/12/12 4:10 PM, Gilles Sadowski wrote: Hello. [For those who don't wish to read the whole post, please at least go towards the end and indicate your preferred option. Thanks.] [...] All are bad arguments violating API contract, all detected at method activation time - IAE. Agreed... if the policy is to _always_ check for null! I am only referring to APIs that explicitly disallow nulls. I don't know what you mean by only: Almost _all_ the API disallows null. What has implicitly or explicitly have to do with that (other than implicitly being equivalent to lacking documentation vs explicitly being defined by a default policy)? I have agreed that we will not always do this. Again, this is one point where we differ as regards to rules. I contend that there must be an ideal, defined by the policy. Is the ideal to always check or to never check. I think the latter, for the reasons explained. The policy cannot be sometimes yes, sometimes no unless you can define in which case to do it and in which not, which is clearly more complicated. The specific case being discussed here is an API that is going to include as an explicit precondition that nulls are not allowed. When this is done, what should be thrown is an IllegalArgumentException, which for us means some subclass of MathIllegalArgumentException. If we were to decide that the ideal is to always check (an obviously different policy), I wouldn't have a problem with this. What I point at is the potential burden on the _user_ if he wants to intercept exceptions: for the _same_ problem (null reference), they'd have to catch two types of exceptions: NPE vs NAE (as sublcass of MIAE). If nobody cares about that, I won't either. I just wanted to stress that we might have some user complaints about this inconsistency which will appear at the contact point between user code and CM code: Because it is simply not nice to report the same problem in different ways. And because it is inconsistent, some future contributor will raise this issue again: Why do we check here and throw MIAE and why don't we check there and let the JVM throw NPE?. [This is exactly what you reported about checking the components of a FieldVector but not the vector itself.] If we sometimes check and sometimes not, the thrown exception will be from a different hierarchy (NPE vs MathRuntimeException). This is annoying (and inconsistent). That is an unfortunate consequence of not always checking. I understand that it may be impractical to always check. Therefore, some NPEs are going to be propagated to clients with no warning. What do you mean by impractical? In practice, we will not check everything tomorrow, but we can decide to work toward that goal. But we have to answer the question not of whether it is practical, but whether it is desirable. This is a quite simple issue; and I don't believe in considerations like it might be good in some part of CM APIs but not in others. [In _principle_ I mean; I don't say that in _practice_ some parts of CM won't be up-to-date sooner than other parts.] The oft-repeated issue is Is it useful to check for null or not?; at least 4 people answered No, because this bug will never go unnoticed: sooner or later, using null _will_ raise exception. The sole and unique difference with CM checking and JVM checking is that the stack trace will (sometimes) be a little shorter in the former case. And inferior first failure data capture and potential additional computation, side effects or other unknown consequences (unless we always think through the consequences of nulls propagated through our code and make sure there are never any untoward side effects. All of these are reasons for the when you must fail, fail fast and loudly maxim. I understand that it may not be possible to adhere to this fully in [math]. You repeat the mantra fail fast, etc. but do not give a counter-argument to mine about the NPE being as fast as it needs to be for this kind of failure. I'd like to see one example of untoward side effects. Most people have come to agree that it's not worth adopting a policy of thorough checking. [Rationale 1: users who encounter a NPE will need to inspect the stack trace and go to _their_ code in order to see where _they_ put a null. Rationale 2: there is probably a performance penalty to all these checks (especially in a well tested code where null reference bugs have been ironed out).] The latter is a good point and why I am OK not checking everywhere. The argument is equally valid _everywhere_ (unless you can provide the above example), in both directions: either it is good to check, then it is always good, or in some cases it is not necessary, then it is never necessary. Consider, e.g. an empty array or array indices that will lead to index out of bounds errors in APIs that take arrays. What is so different about null? There is no difference... if there is a
Re: [Math] About NullArgumentException
On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote: Hi Phil, 2012/9/10 Phil Steitz phil.ste...@gmail.com: On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. I agree with you, it feels weird. I found a better way: we need to make sure that entries of FieldVector can *never* be null. This means checking for null in setters, constructors and the likes. What do you think? That would certainly simplify some code. But (devil's advocate) should we consider that some people may rely on the possibility to set null entries? Gilles [...] - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hi Gilles, 2012/9/12 Gilles Sadowski gil...@harfang.homelinux.org: On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote: Hi Phil, 2012/9/10 Phil Steitz phil.ste...@gmail.com: On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. I agree with you, it feels weird. I found a better way: we need to make sure that entries of FieldVector can *never* be null. This means checking for null in setters, constructors and the likes. What do you think? That would certainly simplify some code. But (devil's advocate) should we consider that some people may rely on the possibility to set null entries? Yes, didn't think of that. However, the javadoc does not specify that null is allowed. So referring to earlier discussions, that should mean that it is forbidden... I'm stretching the argument a little bit, I know. Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/12/12 5:14 AM, Sébastien Brisard wrote: 2012/9/12 Gilles Sadowski gil...@harfang.homelinux.org: On Wed, Sep 12, 2012 at 01:14:32PM +0200, Sébastien Brisard wrote: Hi Gilles, 2012/9/12 Gilles Sadowski gil...@harfang.homelinux.org: On Wed, Sep 12, 2012 at 06:46:29AM +0200, Sébastien Brisard wrote: Hi Phil, 2012/9/10 Phil Steitz phil.ste...@gmail.com: On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. I agree with you, it feels weird. I found a better way: we need to make sure that entries of FieldVector can *never* be null. This means checking for null in setters, constructors and the likes. What do you think? That would certainly simplify some code. But (devil's advocate) should we consider that some people may rely on the possibility to set null entries? Yes, didn't think of that. However, the javadoc does not specify that null is allowed. So referring to earlier discussions, that should mean that it is forbidden... I'm stretching the argument a little bit, I know. That's fine with me. In the sense that it is IMHO a _developer_'s choice: It's a statement about the library (We decide for consistency and robustness reasons, that null is forbidden). I contend that it is the same kind of argument that led most of us to prefer immutable instances (even if some users might want to have mutable ones). Fine, then. I'll implement null checks in all setters and constructors, so that it is guaranteed that null will never be met elsewhere. I will modify the javadoc of the interface to clearly state that null values should not be accepted, and that a MNAE should be thrown in that case. +1 Good idea that adds robustness. It is not ideal, but as a general principle where the javadoc is underspecified, it is IMO fair game to explicitly state preconditions and add checks for them. This amounts to fully specifying the API contract. Code that relies on undocumented behavior can break in this case. This is why it is best to specify the contract as fully as possible. We need to be kind and practical when applying this principle though. Assumptions that *lots of users* are likely to be making made suddenly no longer true should be avoided. The present case is unlikely to be a practical problem for anyone. I know this is a little of a sore subject, but in this and other cases where arguments violate documented preconditions, I think we should throw IAE, which means MNAE is only appropriate as long as it continues to subclass our surrogate for IAE - MIAE. If I sound hard-headed here, it would be great if someone could explain to me what is different about null arguments at the point of method activation in an API that explicitly disallows them. Consider, e.g. an empty array or array indices that will lead to index out of bounds errors in APIs that take arrays. What is so different about null? All are bad arguments violating API contract, all detected at method activation time - IAE. Phil Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
[...] I know this is a little of a sore subject, but in this and other cases where arguments violate documented preconditions, I think we should throw IAE, which means MNAE is only appropriate as long as it continues to subclass our surrogate for IAE - MIAE. If I sound hard-headed here, it would be great if someone could explain to me what is different about null arguments at the point of method activation in an API that explicitly disallows them. Consider, e.g. an empty array or array indices that will lead to index out of bounds errors in APIs that take arrays. What is so different about null? All are bad arguments violating API contract, all detected at method activation time - IAE. Okay, I'm going to lay out again the arguments (all which I know about were already mentioned in the thread). [I'll try to demonstrate that where we differ is on the usefulness of having a policy!] Taking quotes from your mail in reversed order: All are bad arguments violating API contract, all detected at method activation time - IAE. Agreed... if the policy is to _always_ check for null! If we sometimes check and sometimes not, the thrown exception will be from a different hierarchy (NPE vs MathRuntimeException). This is annoying (and inconsistent). The oft-repeated issue is Is it useful to check for null or not?; at least 4 people answered No, because this bug will never go unnoticed: sooner or later, using null _will_ raise exception. The sole and unique difference with CM checking and JVM checking is that the stack trace will (sometimes) be a little shorter in the former case. Most people have come to agree that it's not worth adopting a policy of thorough checking. [Rationale 1: users who encounter a NPE will need to inspect the stack trace and go to _their_ code in order to see where _they_ put a null. Rationale 2: there is probably a performance penalty to all these checks (especially in a well tested code where null reference bugs have been ironed out).] Consider, e.g. an empty array or array indices that will lead to index out of bounds errors in APIs that take arrays. What is so different about null? There is no difference... if there is a policy that decrees so. It is _not_ the policy of the JDK: NullPointerException is not the same error as IndexOutOfBoundsException; their common parent class is the (unspecific) RuntimeException and their sibling is IllegalArgumentException. There is no is-a relationship between NPE, IOBE and IAE. Recalling that some 1.5 years ago you were against the adoption of the single exception hierarchy, rooted at MathRuntimeException, in order to stay faithful to the JDK exception types, I wonder why you are on the opposite stand as NPE is concerned. [...] what is different about null arguments at the point of method activation in an API that explicitly disallows them. The difference is that there is no need to tell the user what the problem is because the raised exception says it all: You tried to use a null reference. [As said above, the only issue is _when_ the exception is triggered.] The policy mandates what is globally valid, e.g.: If not specified otherwise, null is not allowed as an argument. Then, a user cannot complain about a NPE being propagated when he passed an invalid (null) argument. Finally, the case for null is also slightly peculiar (given the above) that it should not simply be bundled with the rationale Fail early: Indeed null references always fail early (i.e. as soon as they are used). Deferring the check until it is done by the JVM will never entails the code to produce a wrong result _because_ of that null reference (it will just fail in the predictable way: NPE).[1] Gilles [1] Unlike in C, where an unitialized pointer would not necessarily crash the program, but _could_ make it behave erratically (including produce wrong results in a stealth way). - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/12/12 8:52 AM, Gilles Sadowski wrote: [...] I know this is a little of a sore subject, but in this and other cases where arguments violate documented preconditions, I think we should throw IAE, which means MNAE is only appropriate as long as it continues to subclass our surrogate for IAE - MIAE. If I sound hard-headed here, it would be great if someone could explain to me what is different about null arguments at the point of method activation in an API that explicitly disallows them. Consider, e.g. an empty array or array indices that will lead to index out of bounds errors in APIs that take arrays. What is so different about null? All are bad arguments violating API contract, all detected at method activation time - IAE. Okay, I'm going to lay out again the arguments (all which I know about were already mentioned in the thread). [I'll try to demonstrate that where we differ is on the usefulness of having a policy!] Taking quotes from your mail in reversed order: All are bad arguments violating API contract, all detected at method activation time - IAE. Agreed... if the policy is to _always_ check for null! I am only referring to APIs that explicitly disallow nulls. I have agreed that we will not always do this. The specific case being discussed here is an API that is going to include as an explicit precondition that nulls are not allowed. When this is done, what should be thrown is an IllegalArgumentException, which for us means some subclass of MathIllegalArgumentException. If we sometimes check and sometimes not, the thrown exception will be from a different hierarchy (NPE vs MathRuntimeException). This is annoying (and inconsistent). That is an unfortunate consequence of not always checking. I understand that it may be impractical to always check. Therefore, some NPEs are going to be propagated to clients with no warning. The oft-repeated issue is Is it useful to check for null or not?; at least 4 people answered No, because this bug will never go unnoticed: sooner or later, using null _will_ raise exception. The sole and unique difference with CM checking and JVM checking is that the stack trace will (sometimes) be a little shorter in the former case. And inferior first failure data capture and potential additional computation, side effects or other unknown consequences (unless we always think through the consequences of nulls propagated through our code and make sure there are never any untoward side effects. All of these are reasons for the when you must fail, fail fast and loudly maxim. I understand that it may not be possible to adhere to this fully in [math]. Most people have come to agree that it's not worth adopting a policy of thorough checking. [Rationale 1: users who encounter a NPE will need to inspect the stack trace and go to _their_ code in order to see where _they_ put a null. Rationale 2: there is probably a performance penalty to all these checks (especially in a well tested code where null reference bugs have been ironed out).] The latter is a good point and why I am OK not checking everywhere. Consider, e.g. an empty array or array indices that will lead to index out of bounds errors in APIs that take arrays. What is so different about null? There is no difference... if there is a policy that decrees so. It is _not_ the policy of the JDK: NullPointerException is not the same error as IndexOutOfBoundsException; their common parent class is the (unspecific) RuntimeException and their sibling is IllegalArgumentException. There is no is-a relationship between NPE, IOBE and IAE. Here you are missing the point above - we are talking about specific APIs that explicitly disallow nulls. Just as we trap or check arguments to avoid array index errors, we can in some cases do the same for nulls. In these cases, it is appropriate to throw IAE for the nulls just as we do for the (avoided) AIOBE. Recalling that some 1.5 years ago you were against the adoption of the single exception hierarchy, rooted at MathRuntimeException, in order to stay faithful to the JDK exception types, I wonder why you are on the opposite stand as NPE is concerned. I am not. I have agreed that we can go back to a common root. But that then forces us to create surrogates for the top level exceptions such as IAE that the math versions used to subclass. I am OK with that. But we should maintain the semantics correctly, throwing a MIAE when API contracts are violated. [...] what is different about null arguments at the point of method activation in an API that explicitly disallows them. The difference is that there is no need to tell the user what the problem is because the raised exception says it all: You tried to use a null reference. [As said above, the only issue is _when_ the exception is triggered.] The policy mandates what is globally valid, e.g.: If not specified otherwise, null is not allowed as an argument. Then, a
Re: [Math] About NullArgumentException
Hi Sébastien. 2012/9/11 Gilles Sadowski gil...@harfang.homelinux.org: On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote: Hi What should I do there? If we adopt the flexible policy (cf. other post), then you can do what you want. ;-) This is absolutely what I do not want to do... I've already realized that I've been too flexible on e.g. formatting this last year... The flexible policy wrt null checks is that we concile the possibility to keep checking (for those who want to), and to not check. And from the user's perspective, both can be handled in the same way (because it is a NPE[1] that will be thrown, sooner or later). [From our perspective, CM will also be consistent albeit, depending on our positions, sometimes incomplete. The advantage of that policy is that, if we indeed want to fail as early as possible, we can just incrementally add precondition checks without changing the overall behaviour of the library: only the stack trace will become shorter...] Regards, Gilles [1] In CM 4.0, when we can change the inheritance tree for NAE. Sébastien I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? Best, Sébastien /** {@inheritDoc} */ public FieldVectorT add(FieldVectorT v) throws DimensionMismatchException { try { return add((ArrayFieldVectorT) v); } catch (ClassCastException cce) { checkVectorDimensions(v); T[] out = buildArray(data.length); for (int i = 0; i data.length; i++) { out[i] = data[i].add(v.getEntry(i)); // SHOULD I CATCH NAE HERE? Not _catch_ NAE but _throw_ it; the line in the loop would become: final T entry = v.getEntry(i); if (entry == null) { throw new NullArgumentException(LocalizedFormats.INDEX, i); } out[i] = data[i].add(entry); } return new ArrayFieldVectorT(field, out, false); } } Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On Tue, Sep 11, 2012 at 07:46:20AM +0200, Sébastien Brisard wrote: Hello, 2012/9/11 Gilles Sadowski gil...@harfang.homelinux.org: On Mon, Sep 10, 2012 at 01:31:12PM -0700, Phil Steitz wrote: On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. Awkward? Of course it is; that's what I explained two posts ago. If we want to allow for the possibility of checking for null (and I agree that it could be useful to pinpoint the problem by passing the information about which index contains an invalid null entry), then we should adopt the second option which I presented in that preceding post: NullArgumentException inherits from NullPointerException This really solves all issues (now that Luc has said that it is not a problem if this one exception escapes the single-root hierarchy): It allows to check for null in CM code and raise the same kind of exception that would arise when no null check is performed. Both flexible and consistent. I may be a bit slow, but I understood that localized error messages were an absolute requirement on Luc's side (which, BTW is good to know, I have always been wondering why we cared so much about localized error messages...). As I've already expressed in the past, I think that localization (as required by some of Luc's applications), as well as the more recent example of passing null to some constructors (as provided by Phil) are indeed to be considered as _outside_ the realm of CM: They are application requirements, pushed down to CM, that cannot be given a self-consistent rationale. When adding a feature to a library like CM, I think that it is very important (namely for long-term maintenance) that it does not rely on out-of-band information. I mean that the Why does this feature exist? should not need references to external-only requirements to be understandable. Of course, by external-only I mean that the feature's implementation could be changed just because the applications that use it changes. E.g. if the TranslatorService, which I imagined in a previous post, existed, Luc's application could use it to fulfill its Error messages should be displayed in French requirement, and the localization of CM error messages could be removed from CM, without CM loosing anything. In that sense, the support of localization is not on the same footing as the support of some math algorithm because the latter is within the core business of CM, while the former is not. IMHO, the more a codebase grows, the less it should contain internal inconsistencies. Of course, we can conclude that now is not the right moment to clean up this or other defects of CM (e.g. because nobody has the time to do it, or to please people who have come to rely on the current state of affairs) but that is not making the issue go away. So, if I understand correctly, NAE inheriting from NPE would mean no error message. No. In the present case, if we can't specify the index of the faulty entry in the error message, You can: throw new NullArgumentException(LocalizedFormats.INDEX, i); I don't see the benefit of NAE over NPE. I must have missed something; I guess that NAE would inherit from NPE, AND implement ExceptionContextProvider, is that right? In that case, I like this solution. Exactly. The _sole_ difference with the current NAE will be the change in the extends clause, from public class NullArgumentException extends MathIllegalArgumentException to public class NullArgumentException extends NullPointerException implements ExceptionContextProvider [And the throw statement above will be the same before and after this change.] Sorry for polluting this thread with silly questions, but I am not used to writing programs for third-party end-users (I *AM* the end-user, or maybe my student enxt door), so I have to say I'm not familiar with the concerns raised in this thread. They are not silly questions. It's precisely my point that such questions will arise (because of CM is lacking self-consistency). I'm pretty sure that none of us is familiar with being only on one side of the border; we all are also _users_ of CM and, quite often, the user's point-of-view is given the priority even if it leads to decisions that are sub-optimal in the longer term. Best regards, Gilles - To unsubscribe, e-mail:
Re: [Math] About NullArgumentException
Hi Phil, 2012/9/10 Phil Steitz phil.ste...@gmail.com: On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. I agree with you, it feels weird. I found a better way: we need to make sure that entries of FieldVector can *never* be null. This means checking for null in setters, constructors and the likes. What do you think? Sébastien Phil Best, Sébastien /** {@inheritDoc} */ public FieldVectorT add(FieldVectorT v) throws DimensionMismatchException { try { return add((ArrayFieldVectorT) v); } catch (ClassCastException cce) { checkVectorDimensions(v); T[] out = buildArray(data.length); for (int i = 0; i data.length; i++) { out[i] = data[i].add(v.getEntry(i)); // SHOULD I CATCH NAE HERE? } return new ArrayFieldVectorT(field, out, false); } } - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hi, 2012/9/10 Gilles Sadowski gil...@harfang.homelinux.org: On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote: On 9/9/12 4:34 AM, Gilles Sadowski wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). Why not just leave things alone [...] For the reason I gave above: the inconsistent/non-existent policy will make the issue recur sooner or later, as it happened now with Sébastien, as it happened with me when I first signalled the burden of checked excpetions and later when we agreed about MathRuntimeException, then changed again, to come back again now, to where we were almost two years ago (IIRC)... - i.e., let some APIs document null handling and throw IAE at the point of method invocation when supplying a null violates the documented API contract? The answer to that question is in the previous post. We can leave the (needless, IMO) NullArgumentException as a subclass of MathIAE in place, or drop it and throw MathIAE directly in these cases. NullArgumentException is about as needless as any subclass of Exception or RuntimeException. Either we use inheritance for what it's primarily meant or we choose another, non-OO, language. This is going in circles; some people will drop from the discussion (or already did) and some time from now, someone will rediscover this, among other little defects of CM. These are worth correcting, but not worth discussing endlessly. Let's just have all people here provide their preference and be done with it. Since there is no way to enforce a strict policy on checking for null in CM, I think that NAE is useless, and should be droped. If we assume that every user of CM can use a debugger, then probably (and contrary to what I advocated earlier), checking early for null is also superfluous. I tend to document arguments which *can* be null (I think someone else also mentioned this practice), so that it's fairly safe (as someone already wrote) to assume that all other arguments *must* be non-null. To sum up, I would favor complete removal of NAE. As for existing checks, I would either remove them, or throw an argumentless standard NPE. Phil was talking about loss of robustness. I don't think that CM as a whole is robust with respect to null pointers. In some places, the code fails in the standard way (NPE), while in others, it fails in a fully documented way. Since this is inconsistent, I don't think we should be afraid of losing robustness in the case of complete removal of existing checks for null. Again, I'm happy to keep them, but I'd rather throw a standard NPE in this case. Sébastien Gilles P.S. Is there an occurrence in CM, where a method can be passed a null argument? Phil The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Le 10/09/2012 08:11, Sébastien Brisard a écrit : Hi, 2012/9/10 Gilles Sadowski gil...@harfang.homelinux.org: On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote: On 9/9/12 4:34 AM, Gilles Sadowski wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). Why not just leave things alone [...] For the reason I gave above: the inconsistent/non-existent policy will make the issue recur sooner or later, as it happened now with Sébastien, as it happened with me when I first signalled the burden of checked excpetions and later when we agreed about MathRuntimeException, then changed again, to come back again now, to where we were almost two years ago (IIRC)... - i.e., let some APIs document null handling and throw IAE at the point of method invocation when supplying a null violates the documented API contract? The answer to that question is in the previous post. We can leave the (needless, IMO) NullArgumentException as a subclass of MathIAE in place, or drop it and throw MathIAE directly in these cases. NullArgumentException is about as needless as any subclass of Exception or RuntimeException. Either we use inheritance for what it's primarily meant or we choose another, non-OO, language. This is going in circles; some people will drop from the discussion (or already did) and some time from now, someone will rediscover this, among other little defects of CM. These are worth correcting, but not worth discussing endlessly. Let's just have all people here provide their preference and be done with it. Since there is no way to enforce a strict policy on checking for null in CM, I think that NAE is useless, and should be droped. If we assume that every user of CM can use a debugger, then probably (and contrary to what I advocated earlier), checking early for null is also superfluous. There are some cases pointed to be Phil which can be useful. We can let them if they are already there. I tend to document arguments which *can* be null (I think someone else also mentioned this practice), so that it's fairly safe (as someone already wrote) to assume that all other arguments *must* be non-null. +1. To sum up, I would favor complete removal of NAE. As for existing checks, I would either remove them, or throw an argumentless standard NPE. As null is so ubiquitous, I would tend to choose Gilles second option (i.e. we preserve NullArgumentException, outside of the single root hierarchy I promote for other exceptions). This would preserve robustness Phil asks for and I would not complain against this as in this case I would not mind if it is not advertised in javadocs and throws declarations (because even if we don't throw it ourselves, a regular NPE from JVM can always occur). Phil was talking about loss of robustness. I don't think that CM as a whole is robust with respect to null pointers. It is not globally robust, but in some places it is because some effort was already put on this. Luc In some places, the code fails in the standard way (NPE), while in others, it fails in a fully documented way. Since this is inconsistent, I don't think we
Re: [Math] About NullArgumentException
[...] P.S. Is there an occurrence in CM, where a method can be passed a null argument? Yes. One example is the constructor for EmpiricalDistribution that takes a RandomGenerator as argument. Thanks for finding one of those few examples. The first remark (concerning the issue at hand in this thread) is that it is enough to say in the documentation of those cases that null is allowed. The policy is (would be) that null is never a valid argument (except in the documented cases). If a null is supplied, the constructor does not complain and the lazy initialization works as though the argumentless constructor had been used and a JDK random generator is created. The second remark is that allowing null in this case only brings confusion because there also exists overloaded constructors. [Overloading is the right way to provide default arguments; using null is to be avoided (and is a remnant from C programming where no two methods can have the same name).] We thus have public EmpiricalDistribution(int binCount) and public EmpiricalDistribution(int binCount, RandomGenerator generator) In the latter case, the doc says: * @param generator random data generator (may be null, resulting in * default JDK generator) Which is also what happens in the former case, albeit a little later. It's confusing because the word lazy does not appear anywhere, so there is no usefulness (from the user's point-of-view) to pass null to the two-args constructor, where he could use the more concise, one-arg, constructor. Even if the doc would highlight the difference, it's still needlessly complicated to allow null since the class's seemingly central method getNextValue would trigger the initialization of the RNG sooner or later. [Hence the gain of lazy init is nil.] If there are use-cases where the RNG part of the functionality is never used, then, IMHO, the design can be improved by separating the data loading + statistics computation from the actual distribution functionality (getNextValue). In the new product, there would be no reference to a RNG (thus vanished the can be null, and if it is, there will be lazy init etc.) in one class, while in the other, getNextValue would indeed be so prominent that the class cannot be used without a RNG (thus lazy init is totally useless) and that argument _cannot_ be null. There are other similar examples, mostly constructors, IIRC. Then a similar analysis probably applies. Gilles Phil Phil The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/10/12 3:44 AM, Gilles Sadowski wrote: [...] P.S. Is there an occurrence in CM, where a method can be passed a null argument? Yes. One example is the constructor for EmpiricalDistribution that takes a RandomGenerator as argument. Thanks for finding one of those few examples. The first remark (concerning the issue at hand in this thread) is that it is enough to say in the documentation of those cases that null is allowed. Good, we agree there. The policy is (would be) that null is never a valid argument (except in the documented cases). I agree there. Where we disagree is what we should do when an illegal null argument is passed to an API that does not accept nulls. I like to document and throw IAE in this case (per guidelines in the Developers Guide). That is more robust, since failure is immediate, there are no potential downstream impacts / resource utilization / leakage potential, and it is easier to debug / trace without looking at the source code, especially if an informative message indicating what precondition failed is provided. If a null is supplied, the constructor does not complain and the lazy initialization works as though the argumentless constructor had been used and a JDK random generator is created. The second remark is that allowing null in this case only brings confusion because there also exists overloaded constructors. [Overloading is the right way to provide default arguments; using null is to be avoided (and is a remnant from C programming where no two methods can have the same name).] I am sure you can figure out how to refactor my applications that use this functionality, but here an example showing why it exists. When initializing an application from external configuration meta-data, if a RandomGenerator class is provided in the config, the initialization framework creates an instance of the provided class and sets a property of an application class being created equal to the instance. This property is used by constructors of other members of the class. If no config is provided for the RandomGenerator, the property is null. The null-safe constructor allows the property to be used as an argument to the constructor, avoiding the need to code (or configure) the test for null in initialization. We thus have public EmpiricalDistribution(int binCount) and public EmpiricalDistribution(int binCount, RandomGenerator generator) In the latter case, the doc says: * @param generator random data generator (may be null, resulting in * default JDK generator) Which is also what happens in the former case, albeit a little later. It's confusing because the word lazy does not appear anywhere, so there is no usefulness (from the user's point-of-view) to pass null to the two-args constructor, where he could use the more concise, one-arg, constructor. See use case above - when what is being passed in is a property, which may be null, it is convenient to be able to provide null to the constructor. Phil Even if the doc would highlight the difference, it's still needlessly complicated to allow null since the class's seemingly central method getNextValue would trigger the initialization of the RNG sooner or later. [Hence the gain of lazy init is nil.] If there are use-cases where the RNG part of the functionality is never used, then, IMHO, the design can be improved by separating the data loading + statistics computation from the actual distribution functionality (getNextValue). In the new product, there would be no reference to a RNG (thus vanished the can be null, and if it is, there will be lazy init etc.) in one class, while in the other, getNextValue would indeed be so prominent that the class cannot be used without a RNG (thus lazy init is totally useless) and that argument _cannot_ be null. There are other similar examples, mostly constructors, IIRC. Then a similar analysis probably applies. Gilles Phil Phil The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/9/12 11:11 PM, Sébastien Brisard wrote: Hi, 2012/9/10 Gilles Sadowski gil...@harfang.homelinux.org: On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote: On 9/9/12 4:34 AM, Gilles Sadowski wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). Why not just leave things alone [...] For the reason I gave above: the inconsistent/non-existent policy will make the issue recur sooner or later, as it happened now with Sébastien, as it happened with me when I first signalled the burden of checked excpetions and later when we agreed about MathRuntimeException, then changed again, to come back again now, to where we were almost two years ago (IIRC)... - i.e., let some APIs document null handling and throw IAE at the point of method invocation when supplying a null violates the documented API contract? The answer to that question is in the previous post. We can leave the (needless, IMO) NullArgumentException as a subclass of MathIAE in place, or drop it and throw MathIAE directly in these cases. NullArgumentException is about as needless as any subclass of Exception or RuntimeException. Either we use inheritance for what it's primarily meant or we choose another, non-OO, language. This is going in circles; some people will drop from the discussion (or already did) and some time from now, someone will rediscover this, among other little defects of CM. These are worth correcting, but not worth discussing endlessly. Let's just have all people here provide their preference and be done with it. Since there is no way to enforce a strict policy on checking for null in CM, I think that NAE is useless, and should be droped. I agree there, as long as APIs that specify null-handling and disallow nulls can throw MathIAE in its place. If we assume that every user of CM can use a debugger, then probably (and contrary to what I advocated earlier), checking early for null is also superfluous. The most important advantage of checking preconditions and throwing client-meaningful exceptions at the point of activation is that it makes it easier to diagnose run time failures - not just for developers debugging code with access to the full sources; but for operations and other developers who may not have access to the sources. Seeing an IllegalArgumentException with an informative message in a stack trace at the point of activation, anyone can look at the published javadoc and understand exactly what happened, at least from the standpoint of the library. Just seeing an undocumented NPE somewhere down in the call stack leads to a harder problem, sometimes requiring added instrumentation, examination of the source code, or other methods to diagnose what is going on. Phil I tend to document arguments which *can* be null (I think someone else also mentioned this practice), so that it's fairly safe (as someone already wrote) to assume that all other arguments *must* be non-null. To sum up, I would favor complete removal of NAE. As for existing checks, I would either remove them, or throw an argumentless standard NPE. Phil was talking about loss of robustness. I don't
Re: [Math] About NullArgumentException
Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? Best, Sébastien /** {@inheritDoc} */ public FieldVectorT add(FieldVectorT v) throws DimensionMismatchException { try { return add((ArrayFieldVectorT) v); } catch (ClassCastException cce) { checkVectorDimensions(v); T[] out = buildArray(data.length); for (int i = 0; i data.length; i++) { out[i] = data[i].add(v.getEntry(i)); // SHOULD I CATCH NAE HERE? } return new ArrayFieldVectorT(field, out, false); } } - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. Phil Best, Sébastien /** {@inheritDoc} */ public FieldVectorT add(FieldVectorT v) throws DimensionMismatchException { try { return add((ArrayFieldVectorT) v); } catch (ClassCastException cce) { checkVectorDimensions(v); T[] out = buildArray(data.length); for (int i = 0; i data.length; i++) { out[i] = data[i].add(v.getEntry(i)); // SHOULD I CATCH NAE HERE? } return new ArrayFieldVectorT(field, out, false); } } - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On Mon, Sep 10, 2012 at 01:31:12PM -0700, Phil Steitz wrote: On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. Awkward? Of course it is; that's what I explained two posts ago. If we want to allow for the possibility of checking for null (and I agree that it could be useful to pinpoint the problem by passing the information about which index contains an invalid null entry), then we should adopt the second option which I presented in that preceding post: NullArgumentException inherits from NullPointerException This really solves all issues (now that Luc has said that it is not a problem if this one exception escapes the single-root hierarchy): It allows to check for null in CM code and raise the same kind of exception that would arise when no null check is performed. Both flexible and consistent. Gilles [...] - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hey, I'm a disinterested third party (not a CM user) but I thought I should chime in my two cents worth... On Sun, Sep 9, 2012 at 4:34 AM, Gilles Sadowski gil...@harfang.homelinux.org wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). I think a standard NPE should be thrown as soon as a null is detected where one shouldn't be. If the caller passes in a null and the callee knows that's bad, throw the NPE right there. Fail fast. Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). I personally think that localizing exceptions is madness. Maybe that's just me. Why not localize the CM source code as well? L10n is a problem for application writers and not math libraries. It seems to me that CM is trying to achieve documentation through exceptions, as if the exceptions thrown should guide the user how to use the library. This is the job of javadoc, tutorials, wikis, etc. For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } Is this going to be the typical case? NPE indicates a programming error, so this is something that *should* completely crash your JVM. However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] +1 this is terrible from an API design perspective In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles Hope you can all work it out. I would start by dropping the document-by-exception thinking that seems to permeate these discussions. Regards, James - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote: Hi What should I do there? If we adopt the flexible policy (cf. other post), then you can do what you want. ;-) I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? Best, Sébastien /** {@inheritDoc} */ public FieldVectorT add(FieldVectorT v) throws DimensionMismatchException { try { return add((ArrayFieldVectorT) v); } catch (ClassCastException cce) { checkVectorDimensions(v); T[] out = buildArray(data.length); for (int i = 0; i data.length; i++) { out[i] = data[i].add(v.getEntry(i)); // SHOULD I CATCH NAE HERE? Not _catch_ NAE but _throw_ it; the line in the loop would become: final T entry = v.getEntry(i); if (entry == null) { throw new NullArgumentException(LocalizedFormats.INDEX, i); } out[i] = data[i].add(entry); } return new ArrayFieldVectorT(field, out, false); } } Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/10/12 3:46 PM, Gilles Sadowski wrote: On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote: Hi What should I do there? If we adopt the flexible policy (cf. other post), then you can do what you want. ;-) Good one :) I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? Best, Sébastien /** {@inheritDoc} */ public FieldVectorT add(FieldVectorT v) throws DimensionMismatchException { try { return add((ArrayFieldVectorT) v); } catch (ClassCastException cce) { checkVectorDimensions(v); T[] out = buildArray(data.length); for (int i = 0; i data.length; i++) { out[i] = data[i].add(v.getEntry(i)); // SHOULD I CATCH NAE HERE? Not _catch_ NAE but _throw_ it; the line in the loop would become: final T entry = v.getEntry(i); if (entry == null) { throw new NullArgumentException(LocalizedFormats.INDEX, i); } out[i] = data[i].add(entry); What about for v itself? Phil } return new ArrayFieldVectorT(field, out, false); } } Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/10/12 3:43 PM, James Ring wrote: Hey, I'm a disinterested third party (not a CM user) but I thought I should chime in my two cents worth... On Sun, Sep 9, 2012 at 4:34 AM, Gilles Sadowski gil...@harfang.homelinux.org wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). I think a standard NPE should be thrown as soon as a null is detected where one shouldn't be. If the caller passes in a null and the callee knows that's bad, throw the NPE right there. Fail fast. Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). I personally think that localizing exceptions is madness. Maybe that's just me. Why not localize the CM source code as well? L10n is a problem for application writers and not math libraries. It seems to me that CM is trying to achieve documentation through exceptions, as if the exceptions thrown should guide the user how to use the library. This is the job of javadoc, tutorials, wikis, etc. For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } Is this going to be the typical case? NPE indicates a programming error, so this is something that *should* completely crash your JVM. Maybe I am being stupid here, but why is NPE so vastly different from, say, array index error and why is it bad to check parameters against documented preconditions and throw IAE when actual parameters do not meet the preconditions? Do you think APIs should just let the chips fall where they may regarding invalid arguments, or do you think checking parameters and throwing IAE with an informative message is an acceptable practice? Phil However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] +1 this is terrible from an API design perspective In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles Hope you can all work it out. I would start by dropping the document-by-exception thinking that seems to permeate these discussions. Regards, James - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hey Phil On Mon, Sep 10, 2012 at 5:09 PM, Phil Steitz phil.ste...@gmail.com wrote: On 9/10/12 3:43 PM, James Ring wrote: try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } Is this going to be the typical case? NPE indicates a programming error, so this is something that *should* completely crash your JVM. Maybe I am being stupid here, but why is NPE so vastly different from, say, array index error and why is it bad to check parameters against documented preconditions and throw IAE when actual parameters do not meet the preconditions? Do you think APIs should just let the chips fall where they may regarding invalid arguments, or do you think checking parameters and throwing IAE with an informative message is an acceptable practice? I don't think NPE is different, you should check parameters against preconditions and throw IAE as soon as you can. APIs should fail fast so the stack traces are meaningful and errors are caught as early as possible. Throwing IAE with an informative error is great. Throwing an exception with a localized message seems pointless to me, FWIW. Phil Regards, James - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hello, 2012/9/11 Gilles Sadowski gil...@harfang.homelinux.org: On Mon, Sep 10, 2012 at 01:31:12PM -0700, Phil Steitz wrote: On 9/10/12 11:47 AM, Sébastien Brisard wrote: Hi What should I do there? I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? IMO, yes. I would also check v itself and add to the javadoc contract that IAE is thrown if v is null. This is not consistently done in [math], though, and rarely in the linear package, so I am OK just letting the NPE propagate if v is null. It is a little awkward that v itself being null leads to NPE, but a component of it null leads to MIAE. Awkward? Of course it is; that's what I explained two posts ago. If we want to allow for the possibility of checking for null (and I agree that it could be useful to pinpoint the problem by passing the information about which index contains an invalid null entry), then we should adopt the second option which I presented in that preceding post: NullArgumentException inherits from NullPointerException This really solves all issues (now that Luc has said that it is not a problem if this one exception escapes the single-root hierarchy): It allows to check for null in CM code and raise the same kind of exception that would arise when no null check is performed. Both flexible and consistent. I may be a bit slow, but I understood that localized error messages were an absolute requirement on Luc's side (which, BTW is good to know, I have always been wondering why we cared so much about localized error messages...). So, if I understand correctly, NAE inheriting from NPE would mean no error message. In the present case, if we can't specify the index of the faulty entry in the error message, I don't see the benefit of NAE over NPE. I must have missed something; I guess that NAE would inherit from NPE, AND implement ExceptionContextProvider, is that right? In that case, I like this solution. Sorry for polluting this thread with silly questions, but I am not used to writing programs for third-party end-users (I *AM* the end-user, or maybe my student enxt door), so I have to say I'm not familiar with the concerns raised in this thread. Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hi, 2012/9/11 Gilles Sadowski gil...@harfang.homelinux.org: On Mon, Sep 10, 2012 at 08:47:35PM +0200, Sébastien Brisard wrote: Hi What should I do there? If we adopt the flexible policy (cf. other post), then you can do what you want. ;-) This is absolutely what I do not want to do... I've already realized that I've been too flexible on e.g. formatting this last year... Sébastien I'm trying to work on MATH-854. It turns out that FieldElementT.add throws a NAE. Should I catch it below, and rethrow it with a more detailed message (including the entry index)? Best, Sébastien /** {@inheritDoc} */ public FieldVectorT add(FieldVectorT v) throws DimensionMismatchException { try { return add((ArrayFieldVectorT) v); } catch (ClassCastException cce) { checkVectorDimensions(v); T[] out = buildArray(data.length); for (int i = 0; i data.length; i++) { out[i] = data[i].add(v.getEntry(i)); // SHOULD I CATCH NAE HERE? Not _catch_ NAE but _throw_ it; the line in the loop would become: final T entry = v.getEntry(i); if (entry == null) { throw new NullArgumentException(LocalizedFormats.INDEX, i); } out[i] = data[i].add(entry); } return new ArrayFieldVectorT(field, out, false); } } Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/9/12 4:34 AM, Gilles Sadowski wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). Why not just leave things alone - i.e., let some APIs document null handling and throw IAE at the point of method invocation when supplying a null violates the documented API contract? We can leave the (needless, IMO) NullArgumentException as a subclass of MathIAE in place, or drop it and throw MathIAE directly in these cases. Phil The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote: On 9/9/12 4:34 AM, Gilles Sadowski wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). Why not just leave things alone [...] For the reason I gave above: the inconsistent/non-existent policy will make the issue recur sooner or later, as it happened now with Sébastien, as it happened with me when I first signalled the burden of checked excpetions and later when we agreed about MathRuntimeException, then changed again, to come back again now, to where we were almost two years ago (IIRC)... - i.e., let some APIs document null handling and throw IAE at the point of method invocation when supplying a null violates the documented API contract? The answer to that question is in the previous post. We can leave the (needless, IMO) NullArgumentException as a subclass of MathIAE in place, or drop it and throw MathIAE directly in these cases. NullArgumentException is about as needless as any subclass of Exception or RuntimeException. Either we use inheritance for what it's primarily meant or we choose another, non-OO, language. This is going in circles; some people will drop from the discussion (or already did) and some time from now, someone will rediscover this, among other little defects of CM. These are worth correcting, but not worth discussing endlessly. Let's just have all people here provide their preference and be done with it. Gilles P.S. Is there an occurrence in CM, where a method can be passed a null argument? Phil The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/9/12 3:26 PM, Gilles Sadowski wrote: On Sun, Sep 09, 2012 at 09:16:51AM -0700, Phil Steitz wrote: On 9/9/12 4:34 AM, Gilles Sadowski wrote: Hi. Further discussion on the JIRA page https://issues.apache.org/jira/browse/MATH-856 cannot reach a consensus on solving the issue that raised this thread. The proposal was that CM never checks for null and lets the JVM do it (and thus throw the standard NPE). Phil wants to retain some null checks but opposes to throwing a NPE without a detailed message. The localization mechanism being implemented in ExceptionContext, we cannot throw a standard NPE (since the error message won't be localized). For a consistent behaviour (as seen from the caller), we would have to implement a subclass of the standard NPE: callers could do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM _or_ by CM). } However, this breaks the consensus we arrived at (for v4.0) about CM throwing only subclasses of MathRuntimeExceprion (singly rooted hierarchy). Phil proposes to throw MathIAE (IMO, it must be the specific NullArgumentException), but this breaks the above use-case: Users have to do try { // Call CM } catch (NullPointerException e) { // Handle NPE (raised by the JVM). } catch (NullArgumentException e) // Handle NPE (raised by CM). } showing blatantly that CM is not consistent: sometimes it lets a JVM exception propagate, and sometimes it catches the problem eralier and throws an exception that is not in the same hierarchy (NPE vs IAE or, in 4.0, MathRuntimeException). This is the current state of affairs, and I think that it is not satisfactory. [As proven by this issue having recurred two or three times already.] In light of this, I propose that either * Phil changes his mind (no check for null performed in CM code), or * we make an exception to the singly-rooted hierarchy just for NullArgumentException (which, in 4.0, would become a subclass of the standard NPE). Why not just leave things alone [...] For the reason I gave above: the inconsistent/non-existent policy will make the issue recur sooner or later, as it happened now with Sébastien, as it happened with me when I first signalled the burden of checked excpetions and later when we agreed about MathRuntimeException, then changed again, to come back again now, to where we were almost two years ago (IIRC)... - i.e., let some APIs document null handling and throw IAE at the point of method invocation when supplying a null violates the documented API contract? The answer to that question is in the previous post. We can leave the (needless, IMO) NullArgumentException as a subclass of MathIAE in place, or drop it and throw MathIAE directly in these cases. NullArgumentException is about as needless as any subclass of Exception or RuntimeException. Either we use inheritance for what it's primarily meant or we choose another, non-OO, language. This is going in circles; some people will drop from the discussion (or already did) and some time from now, someone will rediscover this, among other little defects of CM. These are worth correcting, but not worth discussing endlessly. Let's just have all people here provide their preference and be done with it. Gilles P.S. Is there an occurrence in CM, where a method can be passed a null argument? Yes. One example is the constructor for EmpiricalDistribution that takes a RandomGenerator as argument. If a null is supplied, the constructor does not complain and the lazy initialization works as though the argumentless constructor had been used and a JDK random generator is created. There are other similar examples, mostly constructors, IIRC. Phil Phil The second option cares for all the various positions _except_ the singly-rooted hierarchy. Regards, Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
Yes. One example is the constructor for EmpiricalDistribution that takes a RandomGenerator as argument. If a null is supplied, the constructor does not complain and the lazy initialization works as though the argumentless constructor had been used and a JDK random generator is created. There are other similar examples, mostly constructors, IIRC. Phil Just a thought. What if the instead of binding a generator to the Distribution instances, a uniform double or array of uniform doubles were provided? Cheers, Ole - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)
Hi all, Le 2012-09-06 07:08, Sébastien Brisard a écrit : Hi Gilles, 2012/9/6 sebb seb...@gmail.com: On 5 September 2012 22:46, Gilles Sadowski gil...@harfang.homelinux.org wrote: On Wed, Sep 05, 2012 at 06:30:08PM -, l...@apache.org wrote: Author: luc Date: Wed Sep 5 18:30:08 2012 New Revision: 1381284 URL: http://svn.apache.org/viewvc?rev=1381284view=rev Log: Added throw declarations for package complex. Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284r1=1381283r2=1381284view=diff == --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original) +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep 5 18:30:08 2012 @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.commons.math3.FieldElement; +import org.apache.commons.math3.exception.MathInternalError; import org.apache.commons.math3.exception.NullArgumentException; import org.apache.commons.math3.exception.NotPositiveException; import org.apache.commons.math3.exception.util.LocalizedFormats; @@ -566,12 +567,17 @@ public class Complex implements FieldEle * @since 1.2 */ public Complex acos() { -if (isNaN) { -return NaN; -} +try { +if (isNaN) { +return NaN; +} You are right. It's the result of a bad copy-paste. Regardless of whether it makes sense to catch NUA the above condition should not be part of the try clause. -return this.add(this.sqrt1z().multiply(I)).log() -.multiply(I.negate()); +return this.add(this.sqrt1z().multiply(I)).log() +.multiply(I.negate()); +} catch (NullArgumentException e) { +// this should never happen as intermediat results are not null +throw new MathInternalError(e); +} } Maybe I don't understand the purpose of catching NullArgumentException and rethrowing something else. I agree and did not really liked this. It's clearly a false move by me, sorryr for that. Anyway, I was going to start a new thread about NullArgumentException: my proposal is to never check for null and let the standard NPE be thrown in case of bad usage (null passed where a non-null is required). That would avoid such catch and rethrow for things that never happen. I think it would be better. Our own NullArgumentException don't brings any added value and I'll prefer to let the JVM handle this by itself. I don't remember the arguments and positions of everyone when we discussed it earlier. Best regards, Gilles [...] I was about to start a new thread too, but refrained to do so for lack of knowledge on the history of this particular exception. Check for null is unevenly enforced througout the library, which --to me-- suggests we shouldn't do it at all and contend with NPE. There is one potential use, though. I think we should check for null when the NPE might occur in a different method. This is what happens with new Incrementor(int, MaxCountExceededCallback) : cb is just copied, and fields of cb are invoked elsewhere: in that case, checking for null does make sense. I think even there we could rely on the JVM, for simplicity. Luc Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)
Hi Luc, 2012/9/6 luc l...@spaceroots.org: Hi all, Le 2012-09-06 07:08, Sébastien Brisard a écrit : Hi Gilles, 2012/9/6 sebb seb...@gmail.com: On 5 September 2012 22:46, Gilles Sadowski gil...@harfang.homelinux.org wrote: On Wed, Sep 05, 2012 at 06:30:08PM -, l...@apache.org wrote: Author: luc Date: Wed Sep 5 18:30:08 2012 New Revision: 1381284 URL: http://svn.apache.org/viewvc?rev=1381284view=rev Log: Added throw declarations for package complex. Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284r1=1381283r2=1381284view=diff == --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original) +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep 5 18:30:08 2012 @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.commons.math3.FieldElement; +import org.apache.commons.math3.exception.MathInternalError; import org.apache.commons.math3.exception.NullArgumentException; import org.apache.commons.math3.exception.NotPositiveException; import org.apache.commons.math3.exception.util.LocalizedFormats; @@ -566,12 +567,17 @@ public class Complex implements FieldEle * @since 1.2 */ public Complex acos() { -if (isNaN) { -return NaN; -} +try { +if (isNaN) { +return NaN; +} You are right. It's the result of a bad copy-paste. Regardless of whether it makes sense to catch NUA the above condition should not be part of the try clause. -return this.add(this.sqrt1z().multiply(I)).log() -.multiply(I.negate()); +return this.add(this.sqrt1z().multiply(I)).log() +.multiply(I.negate()); +} catch (NullArgumentException e) { +// this should never happen as intermediat results are not null +throw new MathInternalError(e); +} } Maybe I don't understand the purpose of catching NullArgumentException and rethrowing something else. I agree and did not really liked this. It's clearly a false move by me, sorryr for that. Anyway, I was going to start a new thread about NullArgumentException: my proposal is to never check for null and let the standard NPE be thrown in case of bad usage (null passed where a non-null is required). That would avoid such catch and rethrow for things that never happen. I think it would be better. Our own NullArgumentException don't brings any added value and I'll prefer to let the JVM handle this by itself. I don't remember the arguments and positions of everyone when we discussed it earlier. Best regards, Gilles [...] I was about to start a new thread too, but refrained to do so for lack of knowledge on the history of this particular exception. Check for null is unevenly enforced througout the library, which --to me-- suggests we shouldn't do it at all and contend with NPE. There is one potential use, though. I think we should check for null when the NPE might occur in a different method. This is what happens with new Incrementor(int, MaxCountExceededCallback) : cb is just copied, and fields of cb are invoked elsewhere: in that case, checking for null does make sense. I think even there we could rely on the JVM, for simplicity. Do you mean that we should do nothing, and let NPE occur naturally? The origin of the problem might then be difficult to identify. Or maybe you meant that we check for null in that case, but throw NPE instead of our own MathNullArgument? I would be in favor of the second option. On the other hand it's difficult to check that it's consistently applied throughout our code, and I can see your point in doing nothing. Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)
Le 2012-09-06 13:58, Sébastien Brisard a écrit : Hi Luc, 2012/9/6 luc l...@spaceroots.org: Hi all, Le 2012-09-06 07:08, Sébastien Brisard a écrit : Hi Gilles, 2012/9/6 sebb seb...@gmail.com: On 5 September 2012 22:46, Gilles Sadowski gil...@harfang.homelinux.org wrote: On Wed, Sep 05, 2012 at 06:30:08PM -, l...@apache.org wrote: Author: luc Date: Wed Sep 5 18:30:08 2012 New Revision: 1381284 URL: http://svn.apache.org/viewvc?rev=1381284view=rev Log: Added throw declarations for package complex. Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284r1=1381283r2=1381284view=diff == --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original) +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep 5 18:30:08 2012 @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.commons.math3.FieldElement; +import org.apache.commons.math3.exception.MathInternalError; import org.apache.commons.math3.exception.NullArgumentException; import org.apache.commons.math3.exception.NotPositiveException; import org.apache.commons.math3.exception.util.LocalizedFormats; @@ -566,12 +567,17 @@ public class Complex implements FieldEle * @since 1.2 */ public Complex acos() { -if (isNaN) { -return NaN; -} +try { +if (isNaN) { +return NaN; +} You are right. It's the result of a bad copy-paste. Regardless of whether it makes sense to catch NUA the above condition should not be part of the try clause. -return this.add(this.sqrt1z().multiply(I)).log() -.multiply(I.negate()); +return this.add(this.sqrt1z().multiply(I)).log() +.multiply(I.negate()); +} catch (NullArgumentException e) { +// this should never happen as intermediat results are not null +throw new MathInternalError(e); +} } Maybe I don't understand the purpose of catching NullArgumentException and rethrowing something else. I agree and did not really liked this. It's clearly a false move by me, sorryr for that. Anyway, I was going to start a new thread about NullArgumentException: my proposal is to never check for null and let the standard NPE be thrown in case of bad usage (null passed where a non-null is required). That would avoid such catch and rethrow for things that never happen. I think it would be better. Our own NullArgumentException don't brings any added value and I'll prefer to let the JVM handle this by itself. I don't remember the arguments and positions of everyone when we discussed it earlier. Best regards, Gilles [...] I was about to start a new thread too, but refrained to do so for lack of knowledge on the history of this particular exception. Check for null is unevenly enforced througout the library, which --to me-- suggests we shouldn't do it at all and contend with NPE. There is one potential use, though. I think we should check for null when the NPE might occur in a different method. This is what happens with new Incrementor(int, MaxCountExceededCallback) : cb is just copied, and fields of cb are invoked elsewhere: in that case, checking for null does make sense. I think even there we could rely on the JVM, for simplicity. Do you mean that we should do nothing, and let NPE occur naturally? The origin of the problem might then be difficult to identify. Or maybe you meant that we check for null in that case, but throw NPE instead of our own MathNullArgument? I would be in favor of the second option. On the other hand it's difficult to check that it's consistently applied throughout our code, and I can see your point in doing nothing. I meant to do nothing, and even to not document that NullPointerException can occur. There are simply too many places when users provide objects we use later on, checking everything would be impossible to maintain. There are some cases when passing a null reference is allowed, but they are the minority. They are so rare that I tend to document these specific cases only in the Javadoc, adding a parenthesis (can be null) at the end of the @param. So when nothing is said, it implicitly means to me (cannot be null) and if I pass null, I should not be surprised if something bad happens. best
Re: [Math] About NullArgumentException
Hi. [...] I was about to start a new thread too, but refrained to do so for lack of knowledge on the history of this particular exception. Check for null is unevenly enforced througout the library, which --to me-- suggests we shouldn't do it at all and contend with NPE. There is one potential use, though. I think we should check for null when the NPE might occur in a different method. This is what happens with new Incrementor(int, MaxCountExceededCallback) : cb is just copied, and fields of cb are invoked elsewhere: in that case, checking for null does make sense. I think even there we could rely on the JVM, for simplicity. Do you mean that we should do nothing, and let NPE occur naturally? I'd think we should opt for this. The origin of the problem might then be difficult to identify. That's true. But the reason of the problem is an obvious bug (uninitialized variable). The exception may be raised further down the call sequence, but the stack trace will provide a one way road to discover the source of the problem. Or maybe you meant that we check for null in that case, but throw NPE instead of our own MathNullArgument? No, it sure is simpler to do nothing in CM. :-) Otherwise, we'll have to check every reference (for consistency). I would be in favor of the second option. On the other hand it's difficult to check that it's consistently applied throughout our code, and I can see your point in doing nothing. So, do we agree? Gilles Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
[...] Do you mean that we should do nothing, and let NPE occur naturally? The origin of the problem might then be difficult to identify. Or maybe you meant that we check for null in that case, but throw NPE instead of our own MathNullArgument? I would be in favor of the second option. On the other hand it's difficult to check that it's consistently applied throughout our code, and I can see your point in doing nothing. I meant to do nothing, and even to not document that NullPointerException can occur. There are simply too many places when users provide objects we use later on, checking everything would be impossible to maintain. There are some cases when passing a null reference is allowed, but they are the minority. They are so rare that I tend to document these specific cases only in the Javadoc, adding a parenthesis (can be null) at the end of the @param. So when nothing is said, it implicitly means to me (cannot be null) and if I pass null, I should not be surprised if something bad happens. +1 [Also to be added in the formatting guidelines.] Gilles - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException
On 9/6/12 5:16 AM, Gilles Sadowski wrote: Hi. [...] I was about to start a new thread too, but refrained to do so for lack of knowledge on the history of this particular exception. Check for null is unevenly enforced througout the library, which --to me-- suggests we shouldn't do it at all and contend with NPE. There is one potential use, though. I think we should check for null when the NPE might occur in a different method. This is what happens with new Incrementor(int, MaxCountExceededCallback) : cb is just copied, and fields of cb are invoked elsewhere: in that case, checking for null does make sense. I think even there we could rely on the JVM, for simplicity. Do you mean that we should do nothing, and let NPE occur naturally? I'd think we should opt for this. The origin of the problem might then be difficult to identify. That's true. But the reason of the problem is an obvious bug (uninitialized variable). The exception may be raised further down the call sequence, but the stack trace will provide a one way road to discover the source of the problem. Or maybe you meant that we check for null in that case, but throw NPE instead of our own MathNullArgument? No, it sure is simpler to do nothing in CM. :-) Otherwise, we'll have to check every reference (for consistency). I would be in favor of the second option. On the other hand it's difficult to check that it's consistently applied throughout our code, and I can see your point in doing nothing. So, do we agree? I would rather fully document what the API does on nulls (which we do in a lot of places now), but can live with the go boom for this in some (most?) places if that is the consensus of others. I would prefer at least not to rip out the null checks that we have in a lot of places in the stats package. As a user, I would rather get an IAE with informative message when I pass a not allowed null than an untrapped NPE from somewhere a few layers down in the stack. I would like to preserve the way the code that does null checks today works. Other than that, I am OK dropping the custom [math] exception. Phil Gilles Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[Math] About NullArgumentException (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)
On Wed, Sep 05, 2012 at 06:30:08PM -, l...@apache.org wrote: Author: luc Date: Wed Sep 5 18:30:08 2012 New Revision: 1381284 URL: http://svn.apache.org/viewvc?rev=1381284view=rev Log: Added throw declarations for package complex. Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284r1=1381283r2=1381284view=diff == --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original) +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep 5 18:30:08 2012 @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.commons.math3.FieldElement; +import org.apache.commons.math3.exception.MathInternalError; import org.apache.commons.math3.exception.NullArgumentException; import org.apache.commons.math3.exception.NotPositiveException; import org.apache.commons.math3.exception.util.LocalizedFormats; @@ -566,12 +567,17 @@ public class Complex implements FieldEle * @since 1.2 */ public Complex acos() { -if (isNaN) { -return NaN; -} +try { +if (isNaN) { +return NaN; +} -return this.add(this.sqrt1z().multiply(I)).log() -.multiply(I.negate()); +return this.add(this.sqrt1z().multiply(I)).log() +.multiply(I.negate()); +} catch (NullArgumentException e) { +// this should never happen as intermediat results are not null +throw new MathInternalError(e); +} } Maybe I don't understand the purpose of catching NullArgumentException and rethrowing something else. Anyway, I was going to start a new thread about NullArgumentException: my proposal is to never check for null and let the standard NPE be thrown in case of bad usage (null passed where a non-null is required). That would avoid such catch and rethrow for things that never happen. Best regards, Gilles [...] - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)
On 5 September 2012 22:46, Gilles Sadowski gil...@harfang.homelinux.org wrote: On Wed, Sep 05, 2012 at 06:30:08PM -, l...@apache.org wrote: Author: luc Date: Wed Sep 5 18:30:08 2012 New Revision: 1381284 URL: http://svn.apache.org/viewvc?rev=1381284view=rev Log: Added throw declarations for package complex. Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284r1=1381283r2=1381284view=diff == --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original) +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep 5 18:30:08 2012 @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.commons.math3.FieldElement; +import org.apache.commons.math3.exception.MathInternalError; import org.apache.commons.math3.exception.NullArgumentException; import org.apache.commons.math3.exception.NotPositiveException; import org.apache.commons.math3.exception.util.LocalizedFormats; @@ -566,12 +567,17 @@ public class Complex implements FieldEle * @since 1.2 */ public Complex acos() { -if (isNaN) { -return NaN; -} +try { +if (isNaN) { +return NaN; +} Regardless of whether it makes sense to catch NUA the above condition should not be part of the try clause. -return this.add(this.sqrt1z().multiply(I)).log() -.multiply(I.negate()); +return this.add(this.sqrt1z().multiply(I)).log() +.multiply(I.negate()); +} catch (NullArgumentException e) { +// this should never happen as intermediat results are not null +throw new MathInternalError(e); +} } Maybe I don't understand the purpose of catching NullArgumentException and rethrowing something else. Anyway, I was going to start a new thread about NullArgumentException: my proposal is to never check for null and let the standard NPE be thrown in case of bad usage (null passed where a non-null is required). That would avoid such catch and rethrow for things that never happen. Best regards, Gilles [...] - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [Math] About NullArgumentException (Was: svn commit: r1381284 - in /commons/proper/math/trunk/...)
Hi Gilles, 2012/9/6 sebb seb...@gmail.com: On 5 September 2012 22:46, Gilles Sadowski gil...@harfang.homelinux.org wrote: On Wed, Sep 05, 2012 at 06:30:08PM -, l...@apache.org wrote: Author: luc Date: Wed Sep 5 18:30:08 2012 New Revision: 1381284 URL: http://svn.apache.org/viewvc?rev=1381284view=rev Log: Added throw declarations for package complex. Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexFormat.java commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/ComplexUtils.java Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java?rev=1381284r1=1381283r2=1381284view=diff == --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java (original) +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/complex/Complex.java Wed Sep 5 18:30:08 2012 @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import org.apache.commons.math3.FieldElement; +import org.apache.commons.math3.exception.MathInternalError; import org.apache.commons.math3.exception.NullArgumentException; import org.apache.commons.math3.exception.NotPositiveException; import org.apache.commons.math3.exception.util.LocalizedFormats; @@ -566,12 +567,17 @@ public class Complex implements FieldEle * @since 1.2 */ public Complex acos() { -if (isNaN) { -return NaN; -} +try { +if (isNaN) { +return NaN; +} Regardless of whether it makes sense to catch NUA the above condition should not be part of the try clause. -return this.add(this.sqrt1z().multiply(I)).log() -.multiply(I.negate()); +return this.add(this.sqrt1z().multiply(I)).log() +.multiply(I.negate()); +} catch (NullArgumentException e) { +// this should never happen as intermediat results are not null +throw new MathInternalError(e); +} } Maybe I don't understand the purpose of catching NullArgumentException and rethrowing something else. Anyway, I was going to start a new thread about NullArgumentException: my proposal is to never check for null and let the standard NPE be thrown in case of bad usage (null passed where a non-null is required). That would avoid such catch and rethrow for things that never happen. Best regards, Gilles [...] I was about to start a new thread too, but refrained to do so for lack of knowledge on the history of this particular exception. Check for null is unevenly enforced througout the library, which --to me-- suggests we shouldn't do it at all and contend with NPE. There is one potential use, though. I think we should check for null when the NPE might occur in a different method. This is what happens with new Incrementor(int, MaxCountExceededCallback) : cb is just copied, and fields of cb are invoked elsewhere: in that case, checking for null does make sense. Sébastien - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org