You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mahout.apache.org by "Eugen Paraschiv (JIRA)" <ji...@apache.org> on 2010/08/13 16:56:16 UTC
[jira] Created: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Replace manual precondition checking with Precondition utility class from Guava
-------------------------------------------------------------------------------
Key: MAHOUT-480
URL: https://issues.apache.org/jira/browse/MAHOUT-480
Project: Mahout
Issue Type: Improvement
Components: Clustering
Reporter: Eugen Paraschiv
Fix For: 0.4
Replace manual precondition checking with the Precondition class. This will affect the following classes:
FileDataModel
MemoryDiffStorage
SlopeOneRecommender
FileDiffStorage
AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eugen Paraschiv updated MAHOUT-480:
-----------------------------------
Attachment: MAHOUT-480_v2.patch
This patch covers a much wider portion of the system, in accordance with the feedback
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917316#action_12917316 ]
Sean Owen commented on MAHOUT-480:
----------------------------------
I think this is a fairly heroic effort to use Preconditions across the board. My concern had been about getting into a half-way state where Preconditions was used for some but not all such checking. It's the same sort of issue we'd discussed any number of times about, say, half-using 3 logging systems is worse than none at all, or same for 3 math libraries.
I find this less of an issue here -- not all arg checking can or must be done with Preconditions. We have no existing consistent system to compete with.
And, it looks largely like mahout-core is updated. mahout-examples and mahout-utils are not it seems. I could take that on as well as try to find more instances where it can be used in mahout-core.
mahout-math, perhaps we should not introduce the dependency into.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eugen Paraschiv updated MAHOUT-480:
-----------------------------------
Attachment: MAHOUT-480_v3.patch
The MAHOUT-480_v3.patch contains fixes for all of the mahout-core condition checking that was previously done by hand. A few notes:
- some of the checks are verifying multiple independent conditions on one single line; I left most of those as they were, but they should really be broken into independent checks - I can do that after this patch is integrated
- the guava Preconditions class has more specialized methods for checking various types of conditions; I only used Preconditions.checkArgument() so that the exact type of exception thrown doesn't change (for example some of the other methods throw NullPointerException instead of IllegalArgumentException)
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.5
>
> Attachments: MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eugen Paraschiv updated MAHOUT-480:
-----------------------------------
Priority: Minor (was: Major)
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sean Owen updated MAHOUT-480:
-----------------------------
Status: Resolved (was: Patch Available)
Resolution: Fixed
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Ted Dunning (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ted Dunning updated MAHOUT-480:
-------------------------------
Attachment: MAHOUT-480.patch
Eugen,
I think that there were half a dozen errors in your conversions and at least one error in the original code. Your patch also had some problems being applied on 3 files, probably due to recent commits.
I updated the patch to apply cleanly and I have reviewed all 93 files that you changed, correcting the following kinds of items:
- some tests were not negated correctly
- some of the original checks were backwards
- when data is included in the message, printf syntax should be used to avoid a string construction if the precondition holds
- code should use Mahout style
Can you review this revised patch?
Also, it is good to keep the same name on the file so that JIRA can see earlier versions and deprecate them. This can be very important on projects like Hadoop where patches are reviewed automatically.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.5
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (MAHOUT-480) Replace manual
precondition checking with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12898432#action_12898432 ]
Eugen Paraschiv edited comment on MAHOUT-480 at 8/13/10 5:48 PM:
-----------------------------------------------------------------
This patch covers a much wider portion of the system, in accordance with the feedback. The goal is to replace manual condition checking throwing IllegalArgumentException with Preconditions.checkArguments calls
was (Author: eugenparaschiv):
This patch covers a much wider portion of the system, in accordance with the feedback
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eugen Paraschiv updated MAHOUT-480:
-----------------------------------
Attachment: (was: MAHOUT-480.patch)
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12898349#action_12898349 ]
Sean Owen commented on MAHOUT-480:
----------------------------------
Good one, though the thrust of my message on the dev@ thread was that we ought to do argument checking across the entire codebase this way, or not. I don't want to use Guava in 5 classes only.
But I am at the least happy to make sure that the behavior of the code matches what's in this patch.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917362#action_12917362 ]
Eugen Paraschiv commented on MAHOUT-480:
----------------------------------------
I fully understand your concern about inconsistent state, especially from a conceptual point of view. I will open JIRA issues for mahout-math, mahout-examples and mahout-utils and will provide similar patches for them if that is OK.
>From the API perspective, this kind of cross cutting concern is less of a problem, as the guava checks are perfectly aligned with the manual checks that are in place now, meaning that the type of exception thrown is the same, so incremental changes are less of a problem.
So, now that mahout-core is covered, please tell me how I should go about creating the other JIRA issues so that I can continue with this.
Thanks.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Ted Dunning (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913128#action_12913128 ]
Ted Dunning commented on MAHOUT-480:
------------------------------------
Eugen,
This seems like something that can go into 0.4 if you would like to champion the issue.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sean Owen updated MAHOUT-480:
-----------------------------
Assignee: Sean Owen
Fix Version/s: 0.5
(was: 0.4)
Affects Version/s: 0.3
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.5
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Ted Dunning (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ted Dunning updated MAHOUT-480:
-------------------------------
Fix Version/s: 0.4
(was: 0.5)
I think that this can go in if we get one more pair of eyes on my latest patch.
It may need -p1 to apply. Or -p0. I am using git through IDEA which makes it hard to say which kind of patch is produced.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12913430#action_12913430 ]
Eugen Paraschiv commented on MAHOUT-480:
----------------------------------------
Sure, I will submit a full patch in a few days. Thanks.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917411#action_12917411 ]
Hudson commented on MAHOUT-480:
-------------------------------
Integrated in Mahout-Quality #368 (See [https://hudson.apache.org/hudson/job/Mahout-Quality/368/])
MAHOUT-480
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eugen Paraschiv updated MAHOUT-480:
-----------------------------------
Attachment: MAHOUT-480.patch
This patch contains initial work on this issue - it does not modify the exception logic of the code at all (throws the exact same exception type with the same message). There are still a few instances where the code might benefit from additional checks.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Eugen Paraschiv updated MAHOUT-480:
-----------------------------------
Attachment: (was: MAHOUT-480_v2.patch)
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.5
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (MAHOUT-480) Replace manual
precondition checking with Precondition utility class from Guava
Posted by "Eugen Paraschiv (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12898432#action_12898432 ]
Eugen Paraschiv edited comment on MAHOUT-480 at 8/14/10 6:46 AM:
-----------------------------------------------------------------
This patch covers a much wider portion of the system, in accordance with the feedback. The goal is to replace manual condition checking throwing IllegalArgumentException with Preconditions.checkArguments calls. If the feedback on this is OK, I will of course continue to cover the rest of the system and provide a second patch.
was (Author: eugenparaschiv):
This patch covers a much wider portion of the system, in accordance with the feedback. The goal is to replace manual condition checking throwing IllegalArgumentException with Preconditions.checkArguments calls
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Ted Dunning (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Ted Dunning updated MAHOUT-480:
-------------------------------
Status: Patch Available (was: Open)
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.5
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917377#action_12917377 ]
Sean Owen commented on MAHOUT-480:
----------------------------------
Ted I have the patch locally and have added to it as well as fixed a few more typos. I can commit shortly.
This is a very small point of preference, not one I would insist on, but I like IllegalArgumentException for when you've checked an argument for null and found it to be null, and it can't be. It is perhaps more explicit -- the problem is a bad argument. The description can say what is null. NPE always causes me to guess that there was a developer error inside the library at first glance.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Ted Dunning (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917380#action_12917380 ]
Ted Dunning commented on MAHOUT-480:
------------------------------------
Thanks Sean.
I have the same preference as you do. Argument checking is argument checking and null pointer checking in the code is null pointer checking. The distinction is (slightly) nice to preserve. On the other hand, I would prioritize ease of reading of the exception type, but only if somebody showed me
code that was significantly easier to read.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Ted Dunning (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917376#action_12917376 ]
Ted Dunning commented on MAHOUT-480:
------------------------------------
I agree that leaving out math for now is a good idea.
Has anybody finished checking my patch?
SHould I commit it anyway?
Eugen,
I don't think it is so bad to change the type of exception if it still makes sense. Thus, changing an IllegalArgumentException to a NullPointerException isn't such a bad thing if it makes the code easier to read. It is officially an API change, but a fairly benign one.
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12917363#action_12917363 ]
Sean Owen commented on MAHOUT-480:
----------------------------------
No that's fine, I will do the rest, to match your effort. I don't think we should do mahout-math -- I believe we'd wanted to keep the dependencies there minimal as it is reused by at least one other project. (Well, that was my argument at least.)
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Affects Versions: 0.3
> Reporter: Eugen Paraschiv
> Assignee: Sean Owen
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480.patch, MAHOUT-480_v3.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (MAHOUT-480) Replace manual precondition checking
with Precondition utility class from Guava
Posted by "Sean Owen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MAHOUT-480?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12899898#action_12899898 ]
Sean Owen commented on MAHOUT-480:
----------------------------------
Looking good, are you going further with it?
> Replace manual precondition checking with Precondition utility class from Guava
> -------------------------------------------------------------------------------
>
> Key: MAHOUT-480
> URL: https://issues.apache.org/jira/browse/MAHOUT-480
> Project: Mahout
> Issue Type: Improvement
> Components: Clustering
> Reporter: Eugen Paraschiv
> Priority: Minor
> Fix For: 0.4
>
> Attachments: MAHOUT-480_v2.patch
>
>
> Replace manual precondition checking with the Precondition class. This will affect the following classes:
> FileDataModel
> MemoryDiffStorage
> SlopeOneRecommender
> FileDiffStorage
> AbstractJDBCDiffStorage
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.