You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by Greg Brown <gk...@mac.com> on 2010/09/07 01:32:52 UTC

Re: svn commit: r993167 - /pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java

This should be an IndexOutOfBoundsException, not IllegalArgumentException. In either case, it's not strictly necessary because the same exception will be thrown by the "colors" ArrayList.

On Sep 6, 2010, at 6:26 PM, smartini@apache.org wrote:

> Author: smartini
> Date: Mon Sep  6 22:26:24 2010
> New Revision: 993167
> 
> URL: http://svn.apache.org/viewvc?rev=993167&view=rev
> Log:
> added check range on index in color get/set, so in case of wrong (out of range) index the following Exception will be generated (some parts omitted): Caused by: java.lang.IllegalArgumentException: Index not in range [0, 23]
> 
> Modified:
>    pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java
> 
> Modified: pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java
> URL: http://svn.apache.org/viewvc/pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java?rev=993167&r1=993166&r2=993167&view=diff
> ==============================================================================
> --- pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java (original)
> +++ pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java Mon Sep  6 22:26:24 2010
> @@ -304,6 +304,10 @@ public final class TerraTheme extends Th
>      * @param index
>      */
>     public Color getColor(int index) {
> +        if (index < 0 || index >= colors.getLength()) {
> +            throw new IllegalArgumentException("Index not in range [0, " + (colors.getLength() -1) + "]");
> +        }
> +
>         return colors.get(index);
>     }
> 
> @@ -314,6 +318,9 @@ public final class TerraTheme extends Th
>      * @param color
>      */
>     public void setColor(int index, Color color) {
> +        if (index < 0 || index >= colors.getLength()) {
> +            throw new IllegalArgumentException("Index not in range [0, " + (colors.getLength() -1) + "]");
> +        }
>         if (color == null) {
>             throw new IllegalArgumentException();
>         }
> 
> 


Re: svn commit: r993167 - /pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java

Posted by Sandro Martini <sa...@gmail.com>.
Ok, reverted to previous version.

Sandro

-- 
View this message in context: http://apache-pivot-developers.417237.n3.nabble.com/Re-svn-commit-r993167-pivot-trunk-wtk-terra-src-org-apache-pivot-wtk-skin-terra-TerraTheme-java-tp1429696p1429884.html
Sent from the Apache Pivot - Developers mailing list archive at Nabble.com.

Re: svn commit: r993167 - /pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java

Posted by Greg Brown <gk...@mac.com>.
IMO, it's just a bounds check, and I think the IndexOutOfBounds exception thrown by ArrayList is already pretty clear.

On Sep 6, 2010, at 8:08 PM, Sandro Martini wrote:

> 
> I'm sorry for this, because as seen here
> 
> http://apache-pivot-developers.417237.n3.nabble.com/TerraTheme-methods-on-colors-and-multiplication-factor-s-td1322175.html#a1334055
> 
> I was thinking that this was a little thing to add, at least to give a more
> detailed message.
> 
> So what do you prefer now: revert the fix, or keep and change the exception
> type ? Tell me.
> 
> Note that until the end of the week/start of next week I don't have time to
> do it ... so if you want to do it you are welcome.
> 
> Best regards,
> Sandro
> 
> -- 
> View this message in context: http://apache-pivot-developers.417237.n3.nabble.com/Re-svn-commit-r993167-pivot-trunk-wtk-terra-src-org-apache-pivot-wtk-skin-terra-TerraTheme-java-tp1429696p1429805.html
> Sent from the Apache Pivot - Developers mailing list archive at Nabble.com.


Re: svn commit: r993167 - /pivot/trunk/wtk-terra/src/org/apache/pivot/wtk/skin/terra/TerraTheme.java

Posted by Sandro Martini <sa...@gmail.com>.
I'm sorry for this, because as seen here

http://apache-pivot-developers.417237.n3.nabble.com/TerraTheme-methods-on-colors-and-multiplication-factor-s-td1322175.html#a1334055

I was thinking that this was a little thing to add, at least to give a more
detailed message.

So what do you prefer now: revert the fix, or keep and change the exception
type ? Tell me.

Note that until the end of the week/start of next week I don't have time to
do it ... so if you want to do it you are welcome.

Best regards,
Sandro

-- 
View this message in context: http://apache-pivot-developers.417237.n3.nabble.com/Re-svn-commit-r993167-pivot-trunk-wtk-terra-src-org-apache-pivot-wtk-skin-terra-TerraTheme-java-tp1429696p1429805.html
Sent from the Apache Pivot - Developers mailing list archive at Nabble.com.