commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MATH-646) Unmodifiable views of RealVector
Date Sun, 14 Aug 2011 22:08:27 GMT

    [ https://issues.apache.org/jira/browse/MATH-646?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13084913#comment-13084913
] 

Gilles commented on MATH-646:
-----------------------------

{quote}
I chose not to nest it in AbstractRealVector because it would make the corresponding file
huge.
{quote}

Rather than an issue of large source file, the issue is whether this class should part of
the public API.
Personally I think that it shouldn't because, as discussed on the ML, it is useful from the
perspective of the caller (it cannot be modified by the callee) but is not immutable. A method
signature containing an "UnmodifiableRealVector" parameter could be confusing (leading to
a false sense of security). I'd prefer it to be a private class defined inside "AbstractRealVector.java";
the only way to create instances would be by calling the "unmodifiableRealVector" static method.


In "UnmodifiableEntry", is it necessary to have a constructor?
I'm suspicious that it is possible to call "setIndex" on the supposedly unmodifiable entry.
Maybe that it is harmless (?).
In fact, I must admit that the whole "Entry" hierarchy looks odd to me. Maybe that we should
have closer look at [MATH-626]. I have now the impression that moving the sparse vectors into
their own hierarchy would simplify a lot of methods...

Thanks for the tests; they look quite thorough!

A few things in "UnmodifiableRealVectorAbstractTest":
* The utility method "testMethod" should probably be named something like "doTest" or "callMethod"
to avoid confusion (as by convention "test..." are used for methods called by JUnit). Also,
it should be private.
* Didn't you forget to write the "@Test" annotation for "testGetSubVector"?
* I think that it's better to leave the stack trace printing to JUnit (cf. the exceptions
generated by the "reflection" calls): You could just declare those exceptions in the methods
signature.
* You've excluded "ebeDivide" from the generic test but it is not handled separately.


> Unmodifiable views of RealVector
> --------------------------------
>
>                 Key: MATH-646
>                 URL: https://issues.apache.org/jira/browse/MATH-646
>             Project: Commons Math
>          Issue Type: New Feature
>    Affects Versions: 3.0
>            Reporter: S├ębastien Brisard
>              Labels: linear, vector
>         Attachments: MATH-646.patch
>
>
> The issue has been discussed on the [mailing list|http://mail-archives.apache.org/mod_mbox/commons-dev/201108.mbox/<CAGRH7HqxUb2y1HmFt9VJ-kxsXwipk_MdO0D=rNuazmGPNOTVrA@mail.gmail.com>].
Please find attached a proposal for a new class {{UnmodifiableRealVector}}. I chose not to
nest it in {{AbstractRealVector}} because it would make the corresponding file huge. Therefore,
{{UnmodifiableRealVector}} is {{final}}. Maybe you'd like it to be {{private}} as well? A
static method is provided in {{AbstractRealVector}} to build an {{UnmodifiableRealVector}}
from any {{RealVector}}.
> Tests are also provided. Since iterating through different implementations of {{RealVector}}
is actually different, a test is provided for {{UnmodifiableRealVector}} built on {{ArrayRealVector}}
and {{OpenMapRealVector}}. These tests both derive from the same abstract test class. Hope
everything works fine.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

Mime
View raw message