From: SourceForge.net <no...@so...> - 2011-05-24 11:49:48
|
Patches item #3306775, was opened at 2011-05-24 03:54 Message generated for change (Comment added) made by rajarshi You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3306775&group_id=20024 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: master Group: Needs Review Status: Open Resolution: None Priority: 5 Private: No Submitted By: Jonathan Alvarsson (jonalv) Assigned to: Rajarshi Guha (rajarshi) Summary: fix for bug 3305550 - Pubchemfingerprinter is thread incompa Initial Comment: See attached patch ---------------------------------------------------------------------- >Comment By: Rajarshi Guha (rajarshi) Date: 2011-05-24 07:49 Message: I think it's OK - however I'd prefer that IllegalArgumentException be thrown instead, since the the reason for an exception at that point is an invalid SMARTS. Otherwise good to go ---------------------------------------------------------------------- Comment By: Egon Willighagen (egonw) Date: 2011-05-24 05:15 Message: Looks good to me, and I like the throwing of an IllegalStateException. I am not sure about the effect of moving the instantiation into this place. Rajarshi, please judge that bit. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=320024&aid=3306775&group_id=20024 |