You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Nicolas Malin <ni...@nereide.fr> on 2016/11/30 08:41:07 UTC

Re: svn commit: r1771995 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/DatabaseUtil.java

Hello Paul

Le 30/11/2016 � 07:42, paulfoxworthy@apache.org a �crit :
> @@ -1014,7 +1014,8 @@ public class DatabaseUtil {
>                       // NOTE: this may need a toUpperCase in some cases, keep an eye on it, okay for now just do a compare with equalsIgnoreCase
>                       String tableType = tableSet.getString("TABLE_TYPE");
>                       // only allow certain table types
> -                    if (tableType != null && !"TABLE".equalsIgnoreCase(tableType) && !"VIEW".equalsIgnoreCase(tableType) && !"ALIAS".equalsIgnoreCase(tableType) && !"SYNONYM".equalsIgnoreCase(tableType)) {
> +                    if (tableType != null && !"TABLE".equalsIgnoreCase(tableType) && !"VIEW".equalsIgnoreCase(tableType)
> +                            && !"ALIAS".equalsIgnoreCase(tableType) && !"SYNONYM".equalsIgnoreCase(tableType) && !"BASE TABLE".equalsIgnoreCase(tableType)) {
>                           continue;
>                       }
>   
>
Maybe we can change the if condition by something like that :

if (tableType != null && 
Arrays.asList(types).contains(tableType.toUpperCase()))

Nicolas

Re: svn commit: r1771995 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/Da tabaseUtil.java

Posted by Nicolas Malin <ni...@nereide.fr>.
indeed !


Le 30/11/2016 � 14:43, Jacques Le Roux a �crit :
> Le 30/11/2016 � 09:41, Nicolas Malin a �crit :
>> Hello Paul
>>
>> Le 30/11/2016 � 07:42, paulfoxworthy@apache.org a �crit :
>>> @@ -1014,7 +1014,8 @@ public class DatabaseUtil {
>>>                       // NOTE: this may need a toUpperCase in some 
>>> cases, keep an eye on it, okay for now just do a compare with 
>>> equalsIgnoreCase
>>>                       String tableType = 
>>> tableSet.getString("TABLE_TYPE");
>>>                       // only allow certain table types
>>> -                    if (tableType != null && 
>>> !"TABLE".equalsIgnoreCase(tableType) && 
>>> !"VIEW".equalsIgnoreCase(tableType) && 
>>> !"ALIAS".equalsIgnoreCase(tableType) && 
>>> !"SYNONYM".equalsIgnoreCase(tableType)) {
>>> +                    if (tableType != null && 
>>> !"TABLE".equalsIgnoreCase(tableType) && 
>>> !"VIEW".equalsIgnoreCase(tableType)
>>> +                            && !"ALIAS".equalsIgnoreCase(tableType) 
>>> && !"SYNONYM".equalsIgnoreCase(tableType) && !"BASE 
>>> TABLE".equalsIgnoreCase(tableType)) {
>>>                           continue;
>>>                       }
>>>
>> Maybe we can change the if condition by something like that :
>>
>> if (tableType != null && 
>> Arrays.asList(types).contains(tableType.toUpperCase()))
>>
>> Nicolas
>>
>
> We could even use the Martin Fowler's "Decompose Conditional" 
> refactoring pattern 
> http://www.refactoring.com/catalog/decomposeConditional.html
>
> I mean not only specifically here but all around OFBiz when necessary
>
> While seeing Taher doing a lot of refactoring in the base component I 
> decided it was time for me to read more about it.
> I knew about refactoring from Eclipse tools, but that was it. So I 
> bought http://www.martinfowler.com/books/refactoring.html and I'm now 
> reading it for few weeks.
> I recommend it to everyone who have not the chance to read it yet.
>
> Jacques
>
>


Re: svn commit: r1771995 - /ofbiz/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/jdbc/Da tabaseUtil.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 30/11/2016 � 09:41, Nicolas Malin a �crit :
> Hello Paul
>
> Le 30/11/2016 � 07:42, paulfoxworthy@apache.org a �crit :
>> @@ -1014,7 +1014,8 @@ public class DatabaseUtil {
>>                       // NOTE: this may need a toUpperCase in some cases, keep an eye on it, okay for now just do a compare with equalsIgnoreCase
>>                       String tableType = tableSet.getString("TABLE_TYPE");
>>                       // only allow certain table types
>> -                    if (tableType != null && !"TABLE".equalsIgnoreCase(tableType) && !"VIEW".equalsIgnoreCase(tableType) && 
>> !"ALIAS".equalsIgnoreCase(tableType) && !"SYNONYM".equalsIgnoreCase(tableType)) {
>> +                    if (tableType != null && !"TABLE".equalsIgnoreCase(tableType) && !"VIEW".equalsIgnoreCase(tableType)
>> +                            && !"ALIAS".equalsIgnoreCase(tableType) && !"SYNONYM".equalsIgnoreCase(tableType) && !"BASE 
>> TABLE".equalsIgnoreCase(tableType)) {
>>                           continue;
>>                       }
>>
> Maybe we can change the if condition by something like that :
>
> if (tableType != null && Arrays.asList(types).contains(tableType.toUpperCase()))
>
> Nicolas
>

We could even use the Martin Fowler's "Decompose Conditional" refactoring pattern http://www.refactoring.com/catalog/decomposeConditional.html

I mean not only specifically here but all around OFBiz when necessary

While seeing Taher doing a lot of refactoring in the base component I decided it was time for me to read more about it.
I knew about refactoring from Eclipse tools, but that was it. So I bought http://www.martinfowler.com/books/refactoring.html and I'm now reading it 
for few weeks.
I recommend it to everyone who have not the chance to read it yet.

Jacques