[GitHub] commons-lang issue #345: add jvmLaunchers

2018-08-23 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/345
  
AFAICT the code starts the JVM using an OS process.
It also makes assumptions about the classpath separator character that may 
not apply to all OSes.

There may be other such assumptions. EXEC should already deal with all of 
these.

I still think this belongs in EXEC rather than LANG.

It might make a useful enhancement to EXEC?


---


[GitHub] commons-lang issue #345: add jvmLaunchers

2018-08-23 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/345
  
I think this is getting out of scope for LANG, given that there is already 
the EXEC component.


---


[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...

2018-07-08 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/336#discussion_r200857105
  
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, 
final Random random) {
 swap(array, i - 1, random.nextInt(i), 1);
 }
 }
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns null
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @return The element in the array at the index, or null if it is 
ill-formatted
--- End diff --

What does ill-formatted mean?
Can also return null if the array element is present and null.

I don't think this method is sufficiently useful to be worth adding


---


[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...

2018-07-08 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/336#discussion_r200857070
  
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, 
final Random random) {
 swap(array, i - 1, random.nextInt(i), 1);
 }
 }
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns null
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @return The element in the array at the index, or null if it is 
ill-formatted
+ * @since 3.8
+ */
+public static  T get(T[] array, int index){
+return get(array, index, null);
+}
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns the specified value
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @param defaultReturn the object to be returned if the array is null 
or shorter than the index
+ * @return The element in the array at the specified index, or the 
given argument if it is ill-formatted
+ * @since 3.8
+ */
+public static  T get(T[] array, int index, T defaultReturn){
+if(getLength(array) == 0 || array.length <= index){
+return defaultReturn;
+}
+
+if(index < 0 ){
+index = 0;
+}
+
+return array[index];
+}
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns the specified value
--- End diff --

Does not agree with code


---


[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...

2018-07-08 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/336#discussion_r200857058
  
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, 
final Random random) {
 swap(array, i - 1, random.nextInt(i), 1);
 }
 }
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns null
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @return The element in the array at the index, or null if it is 
ill-formatted
+ * @since 3.8
+ */
+public static  T get(T[] array, int index){
+return get(array, index, null);
+}
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns the specified value
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @param defaultReturn the object to be returned if the array is null 
or shorter than the index
+ * @return The element in the array at the specified index, or the 
given argument if it is ill-formatted
+ * @since 3.8
+ */
+public static  T get(T[] array, int index, T defaultReturn){
+if(getLength(array) == 0 || array.length <= index){
+return defaultReturn;
+}
+
+if(index < 0 ){
+index = 0;
--- End diff --

Surely a negative index should return the default?


---


[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...

2018-07-08 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/336#discussion_r200857077
  
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, 
final Random random) {
 swap(array, i - 1, random.nextInt(i), 1);
 }
 }
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns null
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @return The element in the array at the index, or null if it is 
ill-formatted
+ * @since 3.8
+ */
+public static  T get(T[] array, int index){
+return get(array, index, null);
+}
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns the specified value
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @param defaultReturn the object to be returned if the array is null 
or shorter than the index
+ * @return The element in the array at the specified index, or the 
given argument if it is ill-formatted
+ * @since 3.8
+ */
+public static  T get(T[] array, int index, T defaultReturn){
+if(getLength(array) == 0 || array.length <= index){
+return defaultReturn;
+}
+
+if(index < 0 ){
+index = 0;
+}
+
+return array[index];
+}
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns the specified value
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
--- End diff --

May be null


---


[GitHub] commons-lang pull request #336: LANG-1402: Null/index safe get methods for A...

2018-07-08 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/336#discussion_r200857303
  
--- Diff: src/main/java/org/apache/commons/lang3/ArrayUtils.java ---
@@ -8672,4 +8672,53 @@ public static void shuffle(final double[] array, 
final Random random) {
 swap(array, i - 1, random.nextInt(i), 1);
 }
 }
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns null
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @return The element in the array at the index, or null if it is 
ill-formatted
+ * @since 3.8
+ */
+public static  T get(T[] array, int index){
+return get(array, index, null);
+}
+
+/**
+ * Gets an element from the array if the array is non-null and 
appropriately long, otherwise returns the specified value
+ * @param  the component type of the array
+ * @param array   the array holding the desired element
+ * @param index  the index of the element in the array
+ * @param defaultReturn the object to be returned if the array is null 
or shorter than the index
+ * @return The element in the array at the specified index, or the 
given argument if it is ill-formatted
--- End diff --

ill-formatted?

Again, I don't see the point of this method.
It's a bit more flexible than the original method.
But it's still only useful for the case where you can choose a default 
value that does not appear in the array, and you still have to check whether 
the return value is the default or not.


---


[GitHub] commons-lang issue #336: LANG-1402: Null/index safe get methods for ArrayUti...

2018-07-07 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/336
  
User code has to cast the return to make use of it; that is not ideal.


---


[GitHub] commons-lang issue #331: LANG-1380: FastDateParser too strict on abbreviated...

2018-05-19 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/331
  
The change is hard to review as it contains at least one unrelated change 
and some code re-arrangement.

Would it be possible to provide a PR that addresses only the proposed 
change?


---


[GitHub] commons-lang pull request #269: LANG-1337: Fix test failures in IBM JDK 8 fo...

2017-06-06 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/269#discussion_r120390348
  
--- Diff: 
src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java ---
@@ -316,7 +316,7 @@ public void testReflectionHierarchyArrayList() {
 // representation different for IBM JDK 1.6.0, LANG-727
 assumeFalse("IBM Corporation".equals(SystemUtils.JAVA_VENDOR) && 
"1.6".equals(SystemUtils.JAVA_SPECIFICATION_VERSION));
 assumeFalse("Oracle Corporation".equals(SystemUtils.JAVA_VENDOR) 
&& "1.6".compareTo(SystemUtils.JAVA_SPECIFICATION_VERSION) < 0);
-final List list = new ArrayList<>();
+final List list = new ArrayList<>(10);
--- End diff --

Thanks!

If the test fails when the initial size arg is omitted, does that not also 
affect the behaviour of the method being tested? i.e. do apps also have to 
ensure that they specify the min size when using 
ToStringBuilder.reflectionToString() ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #269: LANG-1337: Fix test failures in IBM JDK 8 fo...

2017-06-06 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/269#discussion_r120385458
  
--- Diff: 
src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java ---
@@ -316,7 +316,7 @@ public void testReflectionHierarchyArrayList() {
 // representation different for IBM JDK 1.6.0, LANG-727
 assumeFalse("IBM Corporation".equals(SystemUtils.JAVA_VENDOR) && 
"1.6".equals(SystemUtils.JAVA_SPECIFICATION_VERSION));
 assumeFalse("Oracle Corporation".equals(SystemUtils.JAVA_VENDOR) 
&& "1.6".compareTo(SystemUtils.JAVA_SPECIFICATION_VERSION) < 0);
-final List list = new ArrayList<>();
+final List list = new ArrayList<>(10);
--- End diff --

I think that needs a comment.
Is the magic number 10 significant?
If so, what determines the value?
Could it ever change?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-26 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
looks good


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-20 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
Also it occurs to me that ProcessorArch and ProcessorType could perhaps be 
subtypes of Processor (Arch and Type). They don't really have an independent 
existence, so do they need separate classes?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-20 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
Agreed it's not necessary to have the isXXX methods.
However it makes the user code shorter and simpler.

Currently the code is:

```
Processor processor = ArchUtils.getProcessor();
if (ProcessorArch.BIT_32.equals(processor.getProcessorArch())) {}
if (ProcessorType.PPC.equals(processor.getProcessorType())) {}
```

It would become:

```
Processor processor = ArchUtils.getProcessor();
if (processor.is32Bit()) {}
if (processor.isPPC()) {}
```

As well as being longer, in the first case one has to remember to whether 
to use ProcessorArch or ProcessorType



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-20 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r106870483
  
--- Diff: src/test/java/org/apache/commons/lang3/ArchUtilsTest.java ---
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import org.apache.commons.lang3.arch.Processor;
+import org.apache.commons.lang3.arch.ProcessorArch;
+import org.junit.Test;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertNotNull;
+
+/**
+ * Test class for {@link ArchUtils}.
+ *
+ * @author Tomschi
+ */
+public class ArchUtilsTest {
+
+// x86
+private static final String X86 = "x86";
+private static final String X86_I386 = "i386";
+private static final String X86_I486 = "i486";
+private static final String X86_I586 = "i586";
+private static final String X86_I686 = "i686";
+private static final String X86_PENTIUM = "pentium";
+
+// x86_64
+private static final String X86_64 = "x86_64";
+private static final String X86_64_AMD64 = "amd64";
+private static final String X86_64_EM64T = "em64t";
+private static final String X86_64_UNIVERSAL = "universal";
+
+// IA64
+private static final String IA64 = "ia64";
+private static final String IA64W = "ia64w";
+
+// IA64_32
+private static final String IA64_32 = "ia64_32";
+private static final String IA64N = "ia64n";
+
+// PPC
+private static final String PPC = "ppc";
+private static final String POWER = "power";
+private static final String POWERPC = "powerpc";
+private static final String POWER_PC = "power_pc";
+private static final String POWER_RS = "power_rs";
+
+// PPC 64
+private static final String PPC64 = "ppc64";
+private static final String POWER64 = "power64";
+private static final String POWERPC64 = "powerpc64";
+private static final String POWER_PC64 = "power_pc64";
+private static final String POWER_RS64 = "power_rs64";
+
+@Test
+public void testIs32BitJVM() {
+Processor processor = ArchUtils.getProcessor(X86);
+assertNotNull(processor);
+
assertTrue(ProcessorArch.BIT_32.equals(processor.getProcessorArch()));
+
+processor = ArchUtils.getProcessor(IA64_32);
+assertNotNull(processor);
+
assertTrue(ProcessorArch.BIT_32.equals(processor.getProcessorArch()));
+
+processor = ArchUtils.getProcessor(PPC);
+assertNotNull(processor);
+
assertTrue(ProcessorArch.BIT_32.equals(processor.getProcessorArch()));
+
+processor = ArchUtils.getProcessor(X86_64);
+assertNotNull(processor);
+
assertFalse(ProcessorArch.BIT_32.equals(processor.getProcessorArch()));
+
+processor = ArchUtils.getProcessor(PPC64);
+assertNotNull(processor);
+
assertFalse(ProcessorArch.BIT_32.equals(processor.getProcessorArch()));
+
+processor = ArchUtils.getProcessor(IA64);
+assertNotNull(processor);
+
assertFalse(ProcessorArch.BIT_32.equals(processor.getProcessorArch()));
+}
+
+@Test
+public void testIs64BitJVM() {
+Processor processor = ArchUtils.getProcessor(X86_64);
+assertNotNull(processor);
+
assertTrue(ProcessorArch.BIT_64.equals(processor.getProcessorArch()));
+
+processor = ArchUtils.getProcessor(PPC64);
+assertNotNull(processor);

[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-15 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
I thought the idea was to move the isXXX methods to the Processor class.

You could then say
```
Processor processor = ArchUtils.getProcessor();
processor.is32Bit();
processor.isPPC();
etc.
```



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-15 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r106312243
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import org.apache.commons.lang3.arch.Processor;
+import org.apache.commons.lang3.arch.ProcessorArch;
+import org.apache.commons.lang3.arch.ProcessorType;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ */
+public class ArchUtils {
+
+private static Map<String, Processor> map;
+
+static {
+map = new HashMap<>();
+init();
+}
+
+private static final void init() {
+init_X86_32Bit();
+init_X86_64Bit();
+init_IA64_32Bit();
+init_IA64_64Bit();
+init_PPC_32Bit();
+init_PPC_64Bit();
+}
+
+/**
+ * Adding x86 32 bit {@link Processor}'s to the map
+ */
+private static final void init_X86_32Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.X86);
+addProcessors(processor, "x86", "i386", "i486", "i586", "i686", 
"pentium");
+}
+
+/**
+ * Adding x86 64 bit {@link Processor}'s to the map
+ */
+private static final void init_X86_64Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.X86);
+addProcessors(processor, "x86_64", "amd64", "em64t", "universal");
+}
+
+/**
+ * Adding ia64 32 bit {@link Processor}'s to the map
+ */
+private static final void init_IA64_32Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.IA_64);
+addProcessors(processor, "ia64_32", "ia64n");
+}
+
+/**
+ * Adding ia64 64 bit {@link Processor}'s to the map
+ */
+private static final void init_IA64_64Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.IA_64);
+addProcessors(processor, "ia64", "ia64w");
+}
+
+/**
+ * Adding PPC 32 bit {@link Processor}'s to the map
+ */
+private static final void init_PPC_32Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.PPC);
+addProcessors(processor, "ppc", "power", "powerpc", "power_pc", 
"power_rs");
+}
+
+/**
+ * Adding PPC 64 bit {@link Processor}'s to the map
+ */
+private static final void init_PPC_64Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.PPC);
+addProcessors(processor, "ppc64", "power64", "powerpc64", 
"power_pc64", "power_rs64");
+}
+
+/**
+ * Adds the given {@link Processor} whith the given key {@link String} 
to the map.
+ *
+ * @param key The key as {@link String}.
+ * @param processor The {@link Processor} to add.
+ * @throws UnsupportedOperationException When key already exists in 
map.
+ */
+private static final void addProcessor(String key, Processor 
processor) throws UnsupportedOperationException {
+if (!map.containsKey(key)) {
+map.put(key, processor);
+} else {
+Str

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-10 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105472960
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,238 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import org.apache.commons.lang3.arch.Processor;
+import org.apache.commons.lang3.arch.ProcessorArch;
+import org.apache.commons.lang3.arch.ProcessorType;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ */
+public class ArchUtils {
+
+private static Map<String, Processor> map;
+
+static {
+map = new HashMap<>();
+init();
+}
+
+private static final void init() {
+init_X86_32Bit();
+init_X86_64Bit();
+init_IA64_32Bit();
+init_IA64_64Bit();
+init_PPC_32Bit();
+init_PPC_64Bit();
+}
+
+/**
+ * Adding x86 32 bit {@link Processor}'s to the map
+ */
+private static final void init_X86_32Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.X86);
+addProcessors(Arrays.asList("x86", "i386", "i486", "i586", "i686", 
"pentium"), processor);
+}
+
+/**
+ * Adding x86 64 bit {@link Processor}'s to the map
+ */
+private static final void init_X86_64Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.X86);
+addProcessors(Arrays.asList("x86_64", "amd64", "em64t", 
"universal"), processor);
+}
+
+/**
+ * Adding ia64 32 bit {@link Processor}'s to the map
+ */
+private static final void init_IA64_32Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.IA_64);
+addProcessors(Arrays.asList("ia64_32", "ia64n"), processor);
+}
+
+/**
+ * Adding ia64 64 bit {@link Processor}'s to the map
+ */
+private static final void init_IA64_64Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.IA_64);
+addProcessors(Arrays.asList("ia64", "ia64w"), processor);
+}
+
+/**
+ * Adding PPC 32 bit {@link Processor}'s to the map
+ */
+private static final void init_PPC_32Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.PPC);
+addProcessors(Arrays.asList("ppc", "power", "powerpc", "power_pc", 
"power_rs"), processor);
+}
+
+/**
+ * Adding PPC 64 bit {@link Processor}'s to the map
+ */
+private static final void init_PPC_64Bit() {
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.PPC);
+addProcessors(Arrays.asList("ppc64", "power64", "powerpc64", 
"power_pc64", "power_rs64"), processor);
+}
+
+/**
+ * Adds the given {@link Processor} whith the given key {@link String} 
to the map.
+ *
+ * @param key The key as {@link String}.
+ * @param processor The {@link Processor} to add.
+ * @throws UnsupportedOperationException When key already exists in 
map.
+ */
+private static final void addProcessor(String key, Processor 
processor) throws Unsuppo

[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-10 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
In which case, I think they belong on the Processor class.

The user code would look like:
```
Processor proc = ArchUtils.getProcessor([String]);
if (proc != null) {
if (proc.isX86() && proc.isPPC()) {
}
} // else not supported
```

At present the same code is

```
if (ArchUtils.isSupported([String])) {
if (ArchUtils.isX86([String]) && ArchUtils.isPPC([String]) )
}
}
```
I think that looks more complicated. 
Also all the isXXX methods have to be overloaded with an optional string 
parameter.
And the map is accessed at least 3 times.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-10 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105468080
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -53,94 +55,76 @@ private static final void init() {
  * Adding x86 32 bit {@link Processor}'s to the map
  */
 private static final void init_X86_32Bit() {
-Processor x86 = new Processor("x86", ProcessorArch.BIT_32, 
ProcessorType.X86);
-Processor i386 = new Processor("i386", ProcessorArch.BIT_32, 
ProcessorType.X86);
-Processor i486 = new Processor("i486", ProcessorArch.BIT_32, 
ProcessorType.X86);
-Processor i586 = new Processor("i586", ProcessorArch.BIT_32, 
ProcessorType.X86);
-Processor i686 = new Processor("i686", ProcessorArch.BIT_32, 
ProcessorType.X86);
-Processor pentium = new Processor("pentium", ProcessorArch.BIT_32, 
ProcessorType.X86);
-addProcessors(x86, i386, i486, i586, i686, pentium);
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.X86);
+addProcessors(Arrays.asList("x86", "i386", "i486", "i586", "i686", 
"pentium"), processor);
 }
 
 /**
  * Adding x86 64 bit {@link Processor}'s to the map
  */
 private static final void init_X86_64Bit() {
-Processor x86_64 = new Processor("x86_64", ProcessorArch.BIT_64, 
ProcessorType.X86);
-Processor amd64 = new Processor("amd64", ProcessorArch.BIT_64, 
ProcessorType.X86);
-Processor em64t = new Processor("em64t", ProcessorArch.BIT_64, 
ProcessorType.X86);
-Processor universal = new Processor("universal", 
ProcessorArch.BIT_64, ProcessorType.X86);
-addProcessors(x86_64, amd64, em64t, universal);
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.X86);
+addProcessors(Arrays.asList("x86_64", "amd64", "em64t", 
"universal"), processor);
 }
 
 /**
  * Adding ia64 32 bit {@link Processor}'s to the map
  */
 private static final void init_IA64_32Bit() {
-Processor ia64_32 = new Processor("ia64_32", ProcessorArch.BIT_32, 
ProcessorType.IA_64);
-Processor ia64n = new Processor("ia64n", ProcessorArch.BIT_32, 
ProcessorType.IA_64);
-addProcessors(ia64_32, ia64n);
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.IA_64);
+addProcessors(Arrays.asList("ia64_32", "ia64n"), processor);
 }
 
 /**
  * Adding ia64 64 bit {@link Processor}'s to the map
  */
 private static final void init_IA64_64Bit() {
-Processor ia64 = new Processor("ia64", ProcessorArch.BIT_64, 
ProcessorType.IA_64);
-Processor ia64w = new Processor("ia64w", ProcessorArch.BIT_64, 
ProcessorType.IA_64);
-addProcessors(ia64, ia64w);
+Processor processor = new Processor(ProcessorArch.BIT_64, 
ProcessorType.IA_64);
+addProcessors(Arrays.asList("ia64", "ia64w"), processor);
 }
 
 /**
  * Adding PPC 32 bit {@link Processor}'s to the map
  */
 private static final void init_PPC_32Bit() {
-Processor ppc = new Processor("ppc", ProcessorArch.BIT_32, 
ProcessorType.PPC);
-Processor power = new Processor("power", ProcessorArch.BIT_32, 
ProcessorType.PPC);
-Processor powerpc = new Processor("powerpc", ProcessorArch.BIT_32, 
ProcessorType.PPC);
-Processor power_pc = new Processor("power_pc", 
ProcessorArch.BIT_32, ProcessorType.PPC);
-Processor power_rs = new Processor("power_rs", 
ProcessorArch.BIT_32, ProcessorType.PPC);
-addProcessors(ppc, power, powerpc, power_pc, power_rs);
+Processor processor = new Processor(ProcessorArch.BIT_32, 
ProcessorType.PPC);
+addProcessors(Arrays.asList("ppc", "power", "powerpc", "power_pc", 
"power_rs"), processor);
 }
 
 /**
  * Adding PPC 64 bit {@link Processor}'s to the map
  */
 private static final void init_PPC_64Bit() {
-Processor ppc64 = new Processor("ppc64", ProcessorArch.BIT_64, 
ProcessorType.PPC);
-Processor power64 = new Processor("power64", ProcessorArch.BIT_64, 
ProcessorType.PPC);
-Processor powerpc64 = new Processor("powerpc64", 
ProcessorArch.BIT_64, ProcessorType.PPC);
-Processor

[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-10 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
There's no point in having the isXXX() methods any more.
There just need to be methods to get the Processor.
If necessary, the Processor class could have isxxx methods on it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-10 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105378662
  
--- Diff: src/main/java/org/apache/commons/lang3/arch/Processor.java ---
@@ -0,0 +1,30 @@
+package org.apache.commons.lang3.arch;
+
+/**
+ *
+ */
+public class Processor {
+
+private String name;
+private ProcessorArch processorArch;
+private ProcessorType processorType;
+
--- End diff --

I don't understand how the name field helps. 
It's obviously necessary to be able to get the Processor object from the 
name, but once you have the object, you know what the name is.
So why do you think the name is necessary?

The name field results in more objects being needed; if the name is not 
included then all the PPC+64 systems (e.g.) can share the same object in the 
init code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-09 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
The new design is more flexible. So one could add Endianness for example.
[The code still does not check for duplicate values for the same key]

However I'm not sure that the chosen names fully agree with the common 
usage.

For example, architecture is often used for CISC, RISC etc.

The enums need to be carefully documented so their domain is clear.

I'm not sure it makes sense to have combined methods such as isPPC64JVM() 
which asserts that it is a PPC and 64bit JVM.

Also there is a usage issue: a false response may mean that the string is 
unknown. Even though there only two ProcessorArch values currently, one cannot 
assume that a string which is not 64 bit must therefore be 32 bit.

I wonder if it would not be simpler to just return the Processor entry.
The caller can then check the attributes in any combinations they want.

If there is no database entry, return a Process instance with enums which 
indicate "Unknown".
(Attribute enums would need an extra 'Unknown' entry).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105178700
  
--- Diff: 
src/main/java/org/apache/commons/lang3/tuple/ArchUtilsImproved.java ---
@@ -0,0 +1,400 @@
+package org.apache.commons.lang3.tuple;
+
+import org.apache.commons.lang3.ArchUtils;
+import org.apache.commons.lang3.SystemUtils;
+import org.apache.commons.lang3.arch.Processor;
+import org.apache.commons.lang3.arch.ProcessorArch;
+import org.apache.commons.lang3.arch.ProcessorType;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ *
+ */
+public class ArchUtilsImproved {
+
+private static Map<String, Processor> map;
+
+static {
+map = new HashMap<>();
+init();
+}
+
+private static final void init() {
+init_X86_32Bit();
+init_X86_64Bit();
+init_IA64_32Bit();
+init_IA64_64Bit();
+init_PPA_32Bit();
+init_PPA_64Bit();
+}
+
+private static final void init_X86_32Bit() {
+Processor x86 = new Processor("x86", ProcessorArch.BIT_32, 
ProcessorType.X86);
+map.put(x86.getName(), x86);
+
+Processor i386 = new Processor("i386", ProcessorArch.BIT_32, 
ProcessorType.X86);
+map.put(i386.getName(), i386);
+
+Processor i486 = new Processor("i486", ProcessorArch.BIT_32, 
ProcessorType.X86);
+map.put(i486.getName(), i486);
+
+Processor i586 = new Processor("i586", ProcessorArch.BIT_32, 
ProcessorType.X86);
+map.put(i586.getName(), i586);
+
+Processor i686 = new Processor("i686", ProcessorArch.BIT_32, 
ProcessorType.X86);
+map.put(i686.getName(), i686);
+
+Processor pentium = new Processor("pentium", ProcessorArch.BIT_32, 
ProcessorType.X86);
+map.put(pentium.getName(), pentium);
+}
+
+private static final void init_X86_64Bit() {
+Processor x86_64 = new Processor("x86_64", ProcessorArch.BIT_64, 
ProcessorType.X86);
+map.put(x86_64.getName(), x86_64);
+
+Processor amd64 = new Processor("amd64", ProcessorArch.BIT_64, 
ProcessorType.X86);
+map.put(amd64.getName(), amd64);
+
+Processor em64t = new Processor("em64t", ProcessorArch.BIT_64, 
ProcessorType.X86);
+map.put(em64t.getName(), em64t);
+
+Processor universal = new Processor("universal", 
ProcessorArch.BIT_64, ProcessorType.X86);
+map.put(universal.getName(), universal);
+}
+
+private static final void init_IA64_32Bit() {
+Processor ia64_32 = new Processor("ia64_32", ProcessorArch.BIT_32, 
ProcessorType.IA_64);
+map.put(ia64_32.getName(), ia64_32);
+
+Processor ia64n = new Processor("ia64n", ProcessorArch.BIT_32, 
ProcessorType.IA_64);
+map.put(ia64n.getName(), ia64n);
+}
+
+private static final void init_IA64_64Bit() {
+Processor ia64 = new Processor("ia64", ProcessorArch.BIT_64, 
ProcessorType.IA_64);
+map.put(ia64.getName(), ia64);
+
+Processor ia64w = new Processor("ia64w", ProcessorArch.BIT_64, 
ProcessorType.IA_64);
+map.put(ia64w.getName(), ia64w);
+}
+
+private static final void init_PPA_32Bit() {
+Processor ppc = new Processor("ppc", ProcessorArch.BIT_32, 
ProcessorType.PPC);
+map.put(ppc.getName(), ppc);
+
+Processor power = new Processor("power", ProcessorArch.BIT_32, 
ProcessorType.PPC);
+map.put(power.getName(), power);
+
+Processor powerpc = new Processor("powerpc", ProcessorArch.BIT_32, 
ProcessorType.PPC);
+map.put(powerpc.getName(), powerpc);
+
+Processor power_pc = new Processor("power_pc", 
ProcessorArch.BIT_32, ProcessorType.PPC);
+map.put(power_pc.getName(), power_pc);
+
+Processor power_rs = new Processor("power_rs", 
ProcessorArch.BIT_32, ProcessorType.PPC);
+map.put(power_rs.getName(), power_rs);
+}
+
+private static final void init_PPA_64Bit() {
+Processor ppc64 = new Processor("ppc64", ProcessorArch.BIT_64, 
ProcessorType.PPC);
+map.put(ppc64.getName(), ppc64);
+
+Processor power64 = new Processor("power64", ProcessorArch.BIT_64, 
ProcessorType.PPC);
+map.put(power64.getName(), power64);
+
+Processor powerpc64 = new Processor("powerpc64", 
ProcessorArch.BIT_64, Processo

[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-09 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r105175128
  
--- Diff: src/main/java/org/apache/commons/lang3/arch/Processor.java ---
@@ -0,0 +1,30 @@
+package org.apache.commons.lang3.arch;
+
+/**
+ *
+ */
+public class Processor {
+
+private String name;
+private ProcessorArch processorArch;
+private ProcessorType processorType;
+
--- End diff --

These should be final.
I don't think the name is needed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-09 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
I wonder why the architecture String constants are not an enum?

Also, the various addxxx() methods don't check if there is already an 
entry, so it's possible to overwrite an existing value. This should be easy to 
fix (I suggest a common method to update the map called by all the others).

===

The current design assumes that a given OS string can only be in one of the 
classifications.
Will that always be the case?
At present checking for 64bit JVMs means knowing which classes are 64 bit 
etc.
It would be possible (but awkward) to add a method for isIntel.
Perhaps consider using separate flags for each attribute, instead of names 
for specific combinations of attributes. This would make it much easier to add 
new attributes, e.g. CISC,RISC


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang issue #231: Evaluate Architecure

2017-03-09 Thread sebbASF
Github user sebbASF commented on the issue:

https://github.com/apache/commons-lang/pull/231
  
If the code is not going to allow updates, then there is no need to use 
ConcurrentHashMap.
If updates are allowed at run-time, there is the possibility that different 
results will be obtained at different times.

Alternatively, it's potentially possible to allow updates before first use 
of the data, but disallow them later.

This is the approach taken in Commons Validator


http://commons.apache.org/proper/commons-validator/xref/org/apache/commons/validator/routines/DomainValidator.html

This uses a flag to allow initial updates.
Users have to get an instance before using the class; this resets the flag 
to stop further changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request #231: Evaluate Architecure

2017-03-07 Thread sebbASF
Github user sebbASF commented on a diff in the pull request:

https://github.com/apache/commons-lang/pull/231#discussion_r104807087
  
--- Diff: src/main/java/org/apache/commons/lang3/ArchUtils.java ---
@@ -0,0 +1,446 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.lang3;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * An utility class for the os.arch System Property. The class defines 
methods for
+ * identifying the architecture of the current JVM.
+ * 
+ * 
+ * Important: The os.arch System Property returns the architecture used by 
the JVM
+ * not of the operating system.
+ * 
+ *
+ * @author Tomschi
+ */
+public class ArchUtils {
+
+/**
+ * This {@link Map} contains the the possible os.arch property {@link 
String}'s.
+ */
+private static final Map<String, String> map;
+
+/**
+ * The value for x86 architecture.
+ */
+private static final String X86 = "x86";
+
+/**
+ * The value for x86_64 architecture.
+ */
+private static final String X86_64 = "x86_64";
+
+/**
+ * The value for ia64_32 architecture.
+ */
+private static final String IA64_32 = "ia64_32";
+
+/**
+ * The value for ia64 architecture.
+ */
+private static final String IA64 = "ia64";
+
+/**
+ * The value for ppc architecture.
+ */
+private static final String PPC = "ppc";
+
+/**
+ * The value for ppc64 architecture.
+ */
+private static final String PPC64 = "ppc64";
+
+static {
+map = new HashMap<>();
+
--- End diff --

HashMap is not thread-safe.
Since there are public methods which can also update the map, this means 
the class is not thread-safe either.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] commons-lang pull request: Lang 1195: Enhance MethodUtils to allow...

2016-05-13 Thread sebbASF
Github user sebbASF commented on the pull request:

https://github.com/apache/commons-lang/pull/141#issuecomment-219006770
  
Note that an alternative solution for testing private methods is to make 
them package-protected.
(and document why they are not private!)

The test code needs to access it from the same package, but that is not 
usually a problem.
If necessary create a public method in the same package which can be used 
by all the test code.
A similar approach can be used for private fields; create a 
package-protected getter.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---