You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ba...@apache.org on 2009/10/18 09:26:00 UTC
svn commit: r826370 - in /commons/proper/lang/trunk/src:
java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
Author: bayard
Date: Sun Oct 18 07:25:59 2009
New Revision: 826370
URL: http://svn.apache.org/viewvc?rev=826370&view=rev
Log:
Implementing an option to UnicodeUnescaper in which the syntax '\u+0047' is supported. By default it remains unsupported to match Java's method of parsing. Request in LANG-507
Modified:
commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
Modified: commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java?rev=826370&r1=826369&r2=826370&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java (original)
+++ commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java Sun Oct 18 07:25:59 2009
@@ -26,6 +26,15 @@
*/
public class UnicodeUnescaper extends CharSequenceTranslator {
+ private boolean escapingPlus = false;
+
+ public void setEscapingPlus(boolean b) {
+ this.escapingPlus = b;
+ }
+ public boolean isEscapingPlus() {
+ return this.escapingPlus;
+ }
+
/**
* {@inheritDoc}
*/
@@ -39,6 +48,13 @@
i++;
}
+ // consume + symbol in \\u+0045
+ if(isEscapingPlus()) {
+ if( (index + i < input.length()) && (input.charAt(index + i) == '+') ) {
+ i++;
+ }
+ }
+
if( (index + i + 4 <= input.length()) ) {
// Get 4 hex digits
CharSequence unicode = input.subSequence(index + i, index + i + 4);
@@ -47,7 +63,7 @@
int value = Integer.parseInt(unicode.toString(), 16);
out.write((char) value);
} catch (NumberFormatException nfe) {
- throw new RuntimeException("Unable to parse unicode value: " + unicode, nfe);
+ throw new IllegalArgumentException("Unable to parse unicode value: " + unicode, nfe);
}
return i + 4;
} else {
Modified: commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java?rev=826370&r1=826369&r2=826370&view=diff
==============================================================================
--- commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java (original)
+++ commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java Sun Oct 18 07:25:59 2009
@@ -27,6 +27,22 @@
*/
public class UnicodeUnescaperTest extends TestCase {
+ // Requested in LANG-507
+ public void testUPlus() throws IOException {
+ UnicodeUnescaper uu = new UnicodeUnescaper();
+
+ String input = "\\u+0047";
+ try {
+ String result = uu.translate(input);
+ fail("Default behaviour should not parse u+");
+ } catch(IllegalArgumentException iae) {
+ // expected
+ }
+
+ uu.setEscapingPlus(true);
+ assertEquals("Failed to unescape unicode characters with 'u+' notation", "G", uu.translate(input));
+ }
+
public void testUuuuu() throws IOException {
UnicodeUnescaper uu = new UnicodeUnescaper();
Re: svn commit: r826370 - in /commons/proper/lang/trunk/src:
java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
Posted by Henri Yandell <fl...@gmail.com>.
Walked away. Pondered. Came back.
s/Param/Option on the below. A Parameter is a user supplied value; an
Option is a user supplied choice. I'm talking about the latter.
On Sun, Oct 18, 2009 at 1:21 PM, Henri Yandell <fl...@gmail.com> wrote:
> Which ended up looking like:
>
> public static enum PARAM { escapePlus };
>
> private EnumSet<PARAM> params;
>
> public UnicodeUnescaper(PARAM... params) {
> if(params.length > 0) {
> this.params = EnumSet.copyOf(Arrays.asList(params));
> }
> }
>
> public boolean isSet(PARAM p) {
> return (params == null) ? false : params.contains(p);
> }
>
> Opinions very, very much desired. Tempted to extend EnumSet to
> simplify a little of the null protection etc. ParamSet<P> and
> protecting against empty arrays and null params. Maybe looking like:
>
> public static enum PARAM { escapePlus };
>
> private ParamSet<PARAM> params = new ParamSet<PARAM>();
>
> public UnicodeUnescaper(PARAM... parameters) {
> this.params.add(parameters);
> }
>
> public boolean isSet(PARAM p) {
> return params.contains(p);
> }
>
> To a user this looks like:
>
> uu = new UnicodeUnescaper(UnicodeUnescaper.PARAM.escapePlus);
>
> with the advantage for us being that we can add options to a class
> without API problems.
>
> Hen
>
> On Sun, Oct 18, 2009 at 1:04 PM, Henri Yandell <fl...@gmail.com> wrote:
>> On Sun, Oct 18, 2009 at 12:49 PM, Henri Yandell <fl...@gmail.com> wrote:
>>
>>> Fair enough on the threading though. I'll move to constructor as I
>>> can't think of anything better.
>>
>> Rambling out loud.
>>
>> Better (for API scaling):
>>
>> enum FooParam { various PARAM options}
>>
>> constructor: Foo(FooParam... fp) { this.options =
>> EnumSet.copyOf(Collections.asList(fp)); }
>>
>> That would work quite nicely to replace all the painful boolean
>> constructor parameters. That leaves us then needing a way to scale
>> this for a single initial parameter:
>>
>> With a recompile, you could have an Object called PARAM and replace it
>> with a FooParam.PARAM later on of type enum:FooParam, but I'm assuming
>> that would be an error at runtime. So for runtime API scaling you
>> would need to set the enum up from the start. Ideally it wants to be
>> in the same class/file though, which means an inner class so you can
>> make it public. So:
>>
>> new Foo(Foo.PARAM.escapingPlus)
>>
>> You could also have a single argument constructor to start with, but
>> again runtime is probably unhappy when you switch (or overload -
>> compile error?) with varargs.
>>
>> Is it a worthwhile pattern to avoid rampant booleanism in a minority
>> of classes? Not convinced.
>>
>> Hen
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r826370 - in /commons/proper/lang/trunk/src:
java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
Posted by Henri Yandell <fl...@gmail.com>.
Which ended up looking like:
public static enum PARAM { escapePlus };
private EnumSet<PARAM> params;
public UnicodeUnescaper(PARAM... params) {
if(params.length > 0) {
this.params = EnumSet.copyOf(Arrays.asList(params));
}
}
public boolean isSet(PARAM p) {
return (params == null) ? false : params.contains(p);
}
Opinions very, very much desired. Tempted to extend EnumSet to
simplify a little of the null protection etc. ParamSet<P> and
protecting against empty arrays and null params. Maybe looking like:
public static enum PARAM { escapePlus };
private ParamSet<PARAM> params = new ParamSet<PARAM>();
public UnicodeUnescaper(PARAM... parameters) {
this.params.add(parameters);
}
public boolean isSet(PARAM p) {
return params.contains(p);
}
To a user this looks like:
uu = new UnicodeUnescaper(UnicodeUnescaper.PARAM.escapePlus);
with the advantage for us being that we can add options to a class
without API problems.
Hen
On Sun, Oct 18, 2009 at 1:04 PM, Henri Yandell <fl...@gmail.com> wrote:
> On Sun, Oct 18, 2009 at 12:49 PM, Henri Yandell <fl...@gmail.com> wrote:
>
>> Fair enough on the threading though. I'll move to constructor as I
>> can't think of anything better.
>
> Rambling out loud.
>
> Better (for API scaling):
>
> enum FooParam { various PARAM options}
>
> constructor: Foo(FooParam... fp) { this.options =
> EnumSet.copyOf(Collections.asList(fp)); }
>
> That would work quite nicely to replace all the painful boolean
> constructor parameters. That leaves us then needing a way to scale
> this for a single initial parameter:
>
> With a recompile, you could have an Object called PARAM and replace it
> with a FooParam.PARAM later on of type enum:FooParam, but I'm assuming
> that would be an error at runtime. So for runtime API scaling you
> would need to set the enum up from the start. Ideally it wants to be
> in the same class/file though, which means an inner class so you can
> make it public. So:
>
> new Foo(Foo.PARAM.escapingPlus)
>
> You could also have a single argument constructor to start with, but
> again runtime is probably unhappy when you switch (or overload -
> compile error?) with varargs.
>
> Is it a worthwhile pattern to avoid rampant booleanism in a minority
> of classes? Not convinced.
>
> Hen
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r826370 - in /commons/proper/lang/trunk/src:
java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
Posted by Henri Yandell <fl...@gmail.com>.
On Sun, Oct 18, 2009 at 12:49 PM, Henri Yandell <fl...@gmail.com> wrote:
> Fair enough on the threading though. I'll move to constructor as I
> can't think of anything better.
Rambling out loud.
Better (for API scaling):
enum FooParam { various PARAM options}
constructor: Foo(FooParam... fp) { this.options =
EnumSet.copyOf(Collections.asList(fp)); }
That would work quite nicely to replace all the painful boolean
constructor parameters. That leaves us then needing a way to scale
this for a single initial parameter:
With a recompile, you could have an Object called PARAM and replace it
with a FooParam.PARAM later on of type enum:FooParam, but I'm assuming
that would be an error at runtime. So for runtime API scaling you
would need to set the enum up from the start. Ideally it wants to be
in the same class/file though, which means an inner class so you can
make it public. So:
new Foo(Foo.PARAM.escapingPlus)
You could also have a single argument constructor to start with, but
again runtime is probably unhappy when you switch (or overload -
compile error?) with varargs.
Is it a worthwhile pattern to avoid rampant booleanism in a minority
of classes? Not convinced.
Hen
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r826370 - in /commons/proper/lang/trunk/src:
java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
Posted by Henri Yandell <fl...@gmail.com>.
On Sun, Oct 18, 2009 at 4:28 AM, sebb <se...@gmail.com> wrote:
> On 18/10/2009, bayard@apache.org <ba...@apache.org> wrote:
>> Author: bayard
>> Date: Sun Oct 18 07:25:59 2009
>> New Revision: 826370
>>
>> URL: http://svn.apache.org/viewvc?rev=826370&view=rev
>> Log:
>> Implementing an option to UnicodeUnescaper in which the syntax '\u+0047' is supported. By default it remains unsupported to match Java's method of parsing. Request in LANG-507
>>
>> Modified:
>> commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
>> commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
>>
>> Modified: commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
>> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java?rev=826370&r1=826369&r2=826370&view=diff
>> ==============================================================================
>> --- commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java (original)
>> +++ commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java Sun Oct 18 07:25:59 2009
>> @@ -26,6 +26,15 @@
>> */
>> public class UnicodeUnescaper extends CharSequenceTranslator {
>>
>> + private boolean escapingPlus = false;
>> +
>> + public void setEscapingPlus(boolean b) {
>> + this.escapingPlus = b;
>> + }
>> + public boolean isEscapingPlus() {
>> + return this.escapingPlus;
>> + }
>> +
>
> Would it not be better to make the class immutable (and thread-safe)
> by providing the setting as a constructor parameter, rather than as
> set/get methods?
Don't know - putting it in the constructor doesn't scale well API-wise
as more options are added. I'm not the biggest fan of that style of
API and it makes for less readable code.
new Thing(true) vs new Thing().setEscapingPlus(true).
2nd is an ugly API, mostly due to some brainwashed need to look like a
Java bean. I'd much rather have: new Thing().escapingPlusOn(). I
guess the constructor approach could be: new Thing(Thing.ESCAPE_PLUS)
leading, potentially, to the stunning C API of: new
Thing(Thing.ESCAPE_PLUS & Thing.ESCAPE_MINUS).
Oh for named parameters: new Thing( escapePlus => true,
escapeMinus => true).
Fair enough on the threading though. I'll move to constructor as I
can't think of anything better.
Hen
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r826370 - in /commons/proper/lang/trunk/src:
java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
Posted by sebb <se...@gmail.com>.
On 18/10/2009, bayard@apache.org <ba...@apache.org> wrote:
> Author: bayard
> Date: Sun Oct 18 07:25:59 2009
> New Revision: 826370
>
> URL: http://svn.apache.org/viewvc?rev=826370&view=rev
> Log:
> Implementing an option to UnicodeUnescaper in which the syntax '\u+0047' is supported. By default it remains unsupported to match Java's method of parsing. Request in LANG-507
>
> Modified:
> commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
> commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
>
> Modified: commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java
> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java?rev=826370&r1=826369&r2=826370&view=diff
> ==============================================================================
> --- commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java (original)
> +++ commons/proper/lang/trunk/src/java/org/apache/commons/lang/text/translate/UnicodeUnescaper.java Sun Oct 18 07:25:59 2009
> @@ -26,6 +26,15 @@
> */
> public class UnicodeUnescaper extends CharSequenceTranslator {
>
> + private boolean escapingPlus = false;
> +
> + public void setEscapingPlus(boolean b) {
> + this.escapingPlus = b;
> + }
> + public boolean isEscapingPlus() {
> + return this.escapingPlus;
> + }
> +
Would it not be better to make the class immutable (and thread-safe)
by providing the setting as a constructor parameter, rather than as
set/get methods?
> /**
> * {@inheritDoc}
> */
> @@ -39,6 +48,13 @@
> i++;
> }
>
> + // consume + symbol in \\u+0045
> + if(isEscapingPlus()) {
> + if( (index + i < input.length()) && (input.charAt(index + i) == '+') ) {
> + i++;
> + }
> + }
> +
> if( (index + i + 4 <= input.length()) ) {
> // Get 4 hex digits
> CharSequence unicode = input.subSequence(index + i, index + i + 4);
> @@ -47,7 +63,7 @@
> int value = Integer.parseInt(unicode.toString(), 16);
> out.write((char) value);
> } catch (NumberFormatException nfe) {
> - throw new RuntimeException("Unable to parse unicode value: " + unicode, nfe);
> + throw new IllegalArgumentException("Unable to parse unicode value: " + unicode, nfe);
> }
> return i + 4;
> } else {
>
> Modified: commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java
> URL: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java?rev=826370&r1=826369&r2=826370&view=diff
> ==============================================================================
> --- commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java (original)
> +++ commons/proper/lang/trunk/src/test/org/apache/commons/lang/text/translate/UnicodeUnescaperTest.java Sun Oct 18 07:25:59 2009
> @@ -27,6 +27,22 @@
> */
> public class UnicodeUnescaperTest extends TestCase {
>
> + // Requested in LANG-507
> + public void testUPlus() throws IOException {
> + UnicodeUnescaper uu = new UnicodeUnescaper();
> +
> + String input = "\\u+0047";
> + try {
> + String result = uu.translate(input);
> + fail("Default behaviour should not parse u+");
> + } catch(IllegalArgumentException iae) {
> + // expected
> + }
> +
> + uu.setEscapingPlus(true);
> + assertEquals("Failed to unescape unicode characters with 'u+' notation", "G", uu.translate(input));
> + }
> +
> public void testUuuuu() throws IOException {
> UnicodeUnescaper uu = new UnicodeUnescaper();
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org