Accessibility scope of variables in Java packages and modules

[Pradhan Yash Sharma]

Hello,

While I was working on UtilCache.java file came across some improvements,
they are as follows:

1) Method and Variable access modifiers can be narrowed down to private
access modifier.

2) Then AtomicLong can be given value 0L instead of 0.

3) Some Variables is used in both synchronized and unsynchronized blocks,
so they can be declared final. eg,



*protected AtomicLong hitCount = new AtomicLong(0);                private
final AtomicLong hitCount = new AtomicLong(0L);*
One variable was able to get most of my attention is

*                protected ConcurrentMap<Object, CacheLine<V>> memoryTable
= null;*

This is used in synchronized and unsynchronized blocks, this Object can be
converted into ThreadLocal or AtomicReference but it would require changes
in the current implementation as well.

Lastly, there is extensive use of for loops for iteration we can use Java 8
Streams, Collector, and other functions to leverage implicit looping
mechanism.


--
Pradhan Yash Sharma

[Jacques Le Roux]

Hi,

Please avoid ThreadLocal as much as possible

For the rest I think a Jira with a patch fits (means sounds OK with me at 1st glance)

Jacques


Le 13/04/2018 à 11:14, Pradhan Yash Sharma a écrit :

> Hello,
>
> While I was working on UtilCache.java file came across some improvements,
> they are as follows:
>
> 1) Method and Variable access modifiers can be narrowed down to private
> access modifier.
>
> 2) Then AtomicLong can be given value 0L instead of 0.
>
> 3) Some Variables is used in both synchronized and unsynchronized blocks,
> so they can be declared final. eg,
>
>
>
> *protected AtomicLong hitCount = new AtomicLong(0);                private
> final AtomicLong hitCount = new AtomicLong(0L);*
> One variable was able to get most of my attention is
>
> *                protected ConcurrentMap<Object, CacheLine<V>> memoryTable
> = null;*
>
> This is used in synchronized and unsynchronized blocks, this Object can be
> converted into ThreadLocal or AtomicReference but it would require changes
> in the current implementation as well.
>
> Lastly, there is extensive use of for loops for iteration we can use Java 8
> Streams, Collector, and other functions to leverage implicit looping
> mechanism.
>
>
> --
> Pradhan Yash Sharma
>

[taher]

Hello Pradhan,

Refactoring is exactly what we need and is a welcomed activity. I
think we should, however, try to avoid "big ideas" across the entire
code base. The subject of your message is the reason why I say that.

So, if you want to start refactoring, I suggest to start with one
piece of code, study it careful, issue a JIRA, and provide a patch.
This should be focused similar to your notes on UtilCache.

On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
<[hidden email]> wrote:

> Hello,
>
> While I was working on UtilCache.java file came across some improvements,
> they are as follows:
>
> 1) Method and Variable access modifiers can be narrowed down to private
> access modifier.
>
> 2) Then AtomicLong can be given value 0L instead of 0.
>
> 3) Some Variables is used in both synchronized and unsynchronized blocks,
> so they can be declared final. eg,
>
>
>
> *protected AtomicLong hitCount = new AtomicLong(0);                private
> final AtomicLong hitCount = new AtomicLong(0L);*
> One variable was able to get most of my attention is
>
> *                protected ConcurrentMap<Object, CacheLine<V>> memoryTable
> = null;*
>
> This is used in synchronized and unsynchronized blocks, this Object can be
> converted into ThreadLocal or AtomicReference but it would require changes
> in the current implementation as well.
>
> Lastly, there is extensive use of for loops for iteration we can use Java 8
> Streams, Collector, and other functions to leverage implicit looping
> mechanism.
>
>
> --
> Pradhan Yash Sharma

[Yash Sharma]

Thank you for the feedback I've created a Jira ticket OFBIZ-10343
<https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add patches
for the same for your review.

Thanks & Regards,
--
*Pradhan Yash Sharma*

On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <[hidden email]
> wrote:

> Hello Pradhan,
>
> Refactoring is exactly what we need and is a welcomed activity. I
> think we should, however, try to avoid "big ideas" across the entire
> code base. The subject of your message is the reason why I say that.
>
> So, if you want to start refactoring, I suggest to start with one
> piece of code, study it careful, issue a JIRA, and provide a patch.
> This should be focused similar to your notes on UtilCache.
>
> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
> <[hidden email]> wrote:
> > Hello,
> >
> > While I was working on UtilCache.java file came across some improvements,
> > they are as follows:
> >
> > 1) Method and Variable access modifiers can be narrowed down to private
> > access modifier.
> >
> > 2) Then AtomicLong can be given value 0L instead of 0.
> >
> > 3) Some Variables is used in both synchronized and unsynchronized blocks,
> > so they can be declared final. eg,
> >
> >
> >
> > *protected AtomicLong hitCount = new AtomicLong(0);
> private
> > final AtomicLong hitCount = new AtomicLong(0L);*
> > One variable was able to get most of my attention is
> >
> > *                protected ConcurrentMap<Object, CacheLine<V>>
> memoryTable
> > = null;*
> >
> > This is used in synchronized and unsynchronized blocks, this Object can
> be
> > converted into ThreadLocal or AtomicReference but it would require
> changes
> > in the current implementation as well.
> >
> > Lastly, there is extensive use of for loops for iteration we can use
> Java 8
> > Streams, Collector, and other functions to leverage implicit looping
> > mechanism.
> >
> >
> > --
> > Pradhan Yash Sharma
>

[Yash Sharma]

Hi Devs,

Here is the detailed information about the things I am working on for
performance optimization in our OFBiz code.

*1.) Downsize Accessibility Scope*
I've tried to downsize accessibility scope of classes, interfaces, abstract
class, declared member variables, enumerations, methods, and constructors
to as minimum as possible as per OFBIz current implementation, still there
is a lot of scope for improvement but it would require changes at the
granular level. I've used this
<https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html> as
my reference point. example:

-    public void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
+    void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);

Limiting the scope of the method from public modifier to package level.

*2.) Using Lambda Expressions*
Then tried to use lambda expressions on simple functional work to leverage
implicit type of coding an example:

-                Map<String, String> initParameters = new LinkedHashMap<>();
-                for (Element e : initParamList) {
-                    initParameters.put(e.getAttribute("name"),
e.getAttribute("value"));
-                }
+                Map<String, String> initParameters =
initParamList.stream().collect(Collectors.toMap(e ->
e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b,
LinkedHashMap::new));


Some of the key benefits of using lambdas will introduce Functional
style over Imperative style
<https://stackoverflow.com/questions/2078978/functional-programming-vs-object-oriented-programming>,
we can use method referencing
<https://docs.oracle.com/javase/tutorial/java/javaOO/methodreferences.html>,
usage of aggregate operations, and it will help developers to write
memory efficient code.


*3.) Using Type Inference*
Java uses type inference so to make code lightweight I've updated
code constructs as shown in the example for more on this refer this article
<https://docs.oracle.com/javase/tutorial/java/generics/genTypeInference.htmlv>
.

-        Map<String, ? extends Object> systemProps =
UtilGenerics.<String, Object> checkMap(System.getProperties());
+        Map<String, ?> systemProps =
UtilGenerics.checkMap(System.getProperties());


*4.) Use of single quote for character*
There is a significant usage of <"Single Character"> in the codebase for
example:

-            throw new GenericConfigException("Error opening file at
location [" + fileUrl.toExternalForm() + "]", e);
+            throw new GenericConfigException("Error opening file at
location [" + fileUrl.toExternalForm() + ']', e);


"]" is comparativlelly slower then ']' Java internally uses Flyweight
Design pattern to create String literals so for every call it will not
create a new Object and used an existing one this will improve
performace to some extend an study can be seen on this
<https://stackoverflow.com/questions/24859500/concatenate-char-literal-x-vs-single-char-string-literal-x>
page.


*5.) Updated Variable Declaration*

Lastly some of the variable declaration is updated this doesn't create
a huge difference but helps JVM at the from implicit conversion.

-        private long cumulativeEvents = 0;
+        private long cumulativeEvents = 0L;


Based on above findings, I have done some code improvement and
provided following patches. *And need community help for reviewing
these changes.*
Kindly provide any improvents or suggestion you have in mind :)


   1. [OFBIZ-10344] Refactoring Variable Scope for
org.apache.ofbiz.base package
<https://issues.apache.org/jira/browse/OFBIZ-10344>
   2. [OFBIZ-10345] Refactoring Variable Scope for
org.apache.ofbiz.catalina.container
<https://issues.apache.org/jira/browse/OFBIZ-10345>
   3. [OFBIZ-10346] Refactoring Variable Scope for
org.apache.ofbiz.common
<https://issues.apache.org/jira/browse/OFBIZ-10346>
   4. [OFBIZ-10347] Refactoring Variable Scope for
org.apache.ofbiz.datafile
<https://issues.apache.org/jira/browse/OFBIZ-10347>
   5. [OFBIZ-10348] Refactoring Variable Scope for
org.apache.ofbiz.entity
<https://issues.apache.org/jira/browse/OFBIZ-10348>


P.S. Apart from this I am also working on performance matrix and will share
it soon.


Thanks & Regards,

--
*Pradhan Yash Sharma*


On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma <[hidden email]
> wrote:

> Thank you for the feedback I've created a Jira ticket OFBIZ-10343
> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add
> patches for the same for your review.
>
> Thanks & Regards,
> --
> *Pradhan Yash Sharma*
>
> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <
> [hidden email]> wrote:
>
>> Hello Pradhan,
>>
>> Refactoring is exactly what we need and is a welcomed activity. I
>> think we should, however, try to avoid "big ideas" across the entire
>> code base. The subject of your message is the reason why I say that.
>>
>> So, if you want to start refactoring, I suggest to start with one
>> piece of code, study it careful, issue a JIRA, and provide a patch.
>> This should be focused similar to your notes on UtilCache.
>>
>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
>> <[hidden email]> wrote:
>> > Hello,
>> >
>> > While I was working on UtilCache.java file came across some
>> improvements,
>> > they are as follows:
>> >
>> > 1) Method and Variable access modifiers can be narrowed down to private
>> > access modifier.
>> >
>> > 2) Then AtomicLong can be given value 0L instead of 0.
>> >
>> > 3) Some Variables is used in both synchronized and unsynchronized
>> blocks,
>> > so they can be declared final. eg,
>> >
>> >
>> >
>> > *protected AtomicLong hitCount = new AtomicLong(0);
>> private
>> > final AtomicLong hitCount = new AtomicLong(0L);*
>> > One variable was able to get most of my attention is
>> >
>> > *                protected ConcurrentMap<Object, CacheLine<V>>
>> memoryTable
>> > = null;*
>> >
>> > This is used in synchronized and unsynchronized blocks, this Object can
>> be
>> > converted into ThreadLocal or AtomicReference but it would require
>> changes
>> > in the current implementation as well.
>> >
>> > Lastly, there is extensive use of for loops for iteration we can use
>> Java 8
>> > Streams, Collector, and other functions to leverage implicit looping
>> > mechanism.
>> >
>> >
>> > --
>> > Pradhan Yash Sharma
>>
>
>

[taher]

Hello Yash,

Thank you for your work on this so far. It's great to see people
focusing on refactoring, which I think should probably be the top
priority for all of us.

I will review the JIRAs some more over the coming days, but I have a
concern that some of the patches are very large.

We had many discussions in the past about focused refactoring vs.
general trends. Focused refactoring means you go after a specific
piece of code like a class or group of related classes / artifacts and
fixing them. General trends, on the other hand, means that you
identify a certain pattern and then making a sweeping change across
the entire code base.

General trends refactorings can be very dangerous, because you are
running after a "trend" not isolating a specific piece of code and
fixing it.

So my recommendation, especially for the bigger patches that you have,
is to redesign / refactor so that it is a topic-based, not
trend-based. We need to make these commits in isolated bite-sized
chunks that focus on a specific area instead of a specific trend (make
public to private, add try-with-resources, or whatever else)

Cheers,
Taher

On Thu, Apr 19, 2018 at 3:24 PM, Yash Sharma
<[hidden email]> wrote:

> Hi Devs,
>
> Here is the detailed information about the things I am working on for
> performance optimization in our OFBiz code.
>
> *1.) Downsize Accessibility Scope*
> I've tried to downsize accessibility scope of classes, interfaces, abstract
> class, declared member variables, enumerations, methods, and constructors
> to as minimum as possible as per OFBIz current implementation, still there
> is a lot of scope for improvement but it would require changes at the
> granular level. I've used this
> <https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html> as
> my reference point. example:
>
> -    public void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
> +    void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
>
> Limiting the scope of the method from public modifier to package level.
>
> *2.) Using Lambda Expressions*
> Then tried to use lambda expressions on simple functional work to leverage
> implicit type of coding an example:
>
> -                Map<String, String> initParameters = new LinkedHashMap<>();
> -                for (Element e : initParamList) {
> -                    initParameters.put(e.getAttribute("name"),
> e.getAttribute("value"));
> -                }
> +                Map<String, String> initParameters =
> initParamList.stream().collect(Collectors.toMap(e ->
> e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b,
> LinkedHashMap::new));
>
>
> Some of the key benefits of using lambdas will introduce Functional
> style over Imperative style
> <https://stackoverflow.com/questions/2078978/functional-programming-vs-object-oriented-programming>,
> we can use method referencing
> <https://docs.oracle.com/javase/tutorial/java/javaOO/methodreferences.html>,
> usage of aggregate operations, and it will help developers to write
> memory efficient code.
>
>
> *3.) Using Type Inference*
> Java uses type inference so to make code lightweight I've updated
> code constructs as shown in the example for more on this refer this article
> <https://docs.oracle.com/javase/tutorial/java/generics/genTypeInference.htmlv>
> .
>
> -        Map<String, ? extends Object> systemProps =
> UtilGenerics.<String, Object> checkMap(System.getProperties());
> +        Map<String, ?> systemProps =
> UtilGenerics.checkMap(System.getProperties());
>
>
> *4.) Use of single quote for character*
> There is a significant usage of <"Single Character"> in the codebase for
> example:
>
> -            throw new GenericConfigException("Error opening file at
> location [" + fileUrl.toExternalForm() + "]", e);
> +            throw new GenericConfigException("Error opening file at
> location [" + fileUrl.toExternalForm() + ']', e);
>
>
> "]" is comparativlelly slower then ']' Java internally uses Flyweight
> Design pattern to create String literals so for every call it will not
> create a new Object and used an existing one this will improve
> performace to some extend an study can be seen on this
> <https://stackoverflow.com/questions/24859500/concatenate-char-literal-x-vs-single-char-string-literal-x>
> page.
>
>
> *5.) Updated Variable Declaration*
>
> Lastly some of the variable declaration is updated this doesn't create
> a huge difference but helps JVM at the from implicit conversion.
>
> -        private long cumulativeEvents = 0;
> +        private long cumulativeEvents = 0L;
>
>
> Based on above findings, I have done some code improvement and
> provided following patches. *And need community help for reviewing
> these changes.*
> Kindly provide any improvents or suggestion you have in mind :)
>
>
>    1. [OFBIZ-10344] Refactoring Variable Scope for
> org.apache.ofbiz.base package
> <https://issues.apache.org/jira/browse/OFBIZ-10344>
>    2. [OFBIZ-10345] Refactoring Variable Scope for
> org.apache.ofbiz.catalina.container
> <https://issues.apache.org/jira/browse/OFBIZ-10345>
>    3. [OFBIZ-10346] Refactoring Variable Scope for
> org.apache.ofbiz.common
> <https://issues.apache.org/jira/browse/OFBIZ-10346>
>    4. [OFBIZ-10347] Refactoring Variable Scope for
> org.apache.ofbiz.datafile
> <https://issues.apache.org/jira/browse/OFBIZ-10347>
>    5. [OFBIZ-10348] Refactoring Variable Scope for
> org.apache.ofbiz.entity
> <https://issues.apache.org/jira/browse/OFBIZ-10348>
>
>
> P.S. Apart from this I am also working on performance matrix and will share
> it soon.
>
>
> Thanks & Regards,
>
> --
> *Pradhan Yash Sharma*
>
>
> On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma <[hidden email]
>> wrote:
>
>> Thank you for the feedback I've created a Jira ticket OFBIZ-10343
>> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add
>> patches for the same for your review.
>>
>> Thanks & Regards,
>> --
>> *Pradhan Yash Sharma*
>>
>> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <
>> [hidden email]> wrote:
>>
>>> Hello Pradhan,
>>>
>>> Refactoring is exactly what we need and is a welcomed activity. I
>>> think we should, however, try to avoid "big ideas" across the entire
>>> code base. The subject of your message is the reason why I say that.
>>>
>>> So, if you want to start refactoring, I suggest to start with one
>>> piece of code, study it careful, issue a JIRA, and provide a patch.
>>> This should be focused similar to your notes on UtilCache.
>>>
>>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
>>> <[hidden email]> wrote:
>>> > Hello,
>>> >
>>> > While I was working on UtilCache.java file came across some
>>> improvements,
>>> > they are as follows:
>>> >
>>> > 1) Method and Variable access modifiers can be narrowed down to private
>>> > access modifier.
>>> >
>>> > 2) Then AtomicLong can be given value 0L instead of 0.
>>> >
>>> > 3) Some Variables is used in both synchronized and unsynchronized
>>> blocks,
>>> > so they can be declared final. eg,
>>> >
>>> >
>>> >
>>> > *protected AtomicLong hitCount = new AtomicLong(0);
>>> private
>>> > final AtomicLong hitCount = new AtomicLong(0L);*
>>> > One variable was able to get most of my attention is
>>> >
>>> > *                protected ConcurrentMap<Object, CacheLine<V>>
>>> memoryTable
>>> > = null;*
>>> >
>>> > This is used in synchronized and unsynchronized blocks, this Object can
>>> be
>>> > converted into ThreadLocal or AtomicReference but it would require
>>> changes
>>> > in the current implementation as well.
>>> >
>>> > Lastly, there is extensive use of for loops for iteration we can use
>>> Java 8
>>> > Streams, Collector, and other functions to leverage implicit looping
>>> > mechanism.
>>> >
>>> >
>>> > --
>>> > Pradhan Yash Sharma
>>>
>>
>>

[Yash Sharma]

Hello,

Thank you for the response, I was wondering about the volume of refactoring
we can do at each component, so let's apply Divide and Conquer approach for
each component upgrading work.

I can see a few patterns for the update which I've listed down in my
previous mail. We can pick any piece of code and apply focused refactoring
on each component, and then we can do it with others as well when we are
through with one. It would be a great help if you could suggest a sequence
to do so for example :
*I --> 4.) Use of single quote for a character *(this is a straightforward
work and the easiest one :) ).
*II --> **3.) Using Type Inference *(We can pick this as it will never
impact any working code).
III --> *5.) Updated Variable Declaration.*
*IV --> **1.) **Downsize Accessibility Scope *(If tested this is also a not
a big deal).
*V --> **2.) Using **Lambda Expressions *(This can impact on working hence
put at last)

This is what I can think :) Please mentor me on this and suggest any better
action plan we can opt for.

I am very much excited to work on and implement some really cool things
that Java Ecosystem can offer us like Functional Programming, Jigsaw
<http://openjdk.java.net/projects/jigsaw/>, or Local Variable Type Inference
<https://developer.oracle.com/java/jdk-10-local-variable-type-inference>, I
am not penning dowing


Thanks & Regards,
--
*Pradhan Yash Sharma*

On Sat, Apr 28, 2018 at 6:59 PM, Taher Alkhateeb <[hidden email]
> wrote:

> Hello Yash,
>
> Thank you for your work on this so far. It's great to see people
> focusing on refactoring, which I think should probably be the top
> priority for all of us.
>
> I will review the JIRAs some more over the coming days, but I have a
> concern that some of the patches are very large.
>
> We had many discussions in the past about focused refactoring vs.
> general trends. Focused refactoring means you go after a specific
> piece of code like a class or group of related classes / artifacts and
> fixing them. General trends, on the other hand, means that you
> identify a certain pattern and then making a sweeping change across
> the entire code base.
>
> General trends refactorings can be very dangerous, because you are
> running after a "trend" not isolating a specific piece of code and
> fixing it.
>
> So my recommendation, especially for the bigger patches that you have,
> is to redesign / refactor so that it is a topic-based, not
> trend-based. We need to make these commits in isolated bite-sized
> chunks that focus on a specific area instead of a specific trend (make
> public to private, add try-with-resources, or whatever else)
>
> Cheers,
> Taher
>
> On Thu, Apr 19, 2018 at 3:24 PM, Yash Sharma
> <[hidden email]> wrote:
> > Hi Devs,
> >
> > Here is the detailed information about the things I am working on for
> > performance optimization in our OFBiz code.
> >
> > *1.) Downsize Accessibility Scope*
> > I've tried to downsize accessibility scope of classes, interfaces,
> abstract
> > class, declared member variables, enumerations, methods, and constructors
> > to as minimum as possible as per OFBIz current implementation, still
> there
> > is a lot of scope for improvement but it would require changes at the
> > granular level. I've used this
> > <https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html>
> as
> > my reference point. example:
> >
> > -    public void noteKeyRemoval(UtilCache<K, V> cache, K key, V
> oldValue);
> > +    void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
> >
> > Limiting the scope of the method from public modifier to package level.
> >
> > *2.) Using Lambda Expressions*
> > Then tried to use lambda expressions on simple functional work to
> leverage
> > implicit type of coding an example:
> >
> > -                Map<String, String> initParameters = new
> LinkedHashMap<>();
> > -                for (Element e : initParamList) {
> > -                    initParameters.put(e.getAttribute("name"),
> > e.getAttribute("value"));
> > -                }
> > +                Map<String, String> initParameters =
> > initParamList.stream().collect(Collectors.toMap(e ->
> > e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b,
> > LinkedHashMap::new));
> >
> >
> > Some of the key benefits of using lambdas will introduce Functional
> > style over Imperative style
> > <https://stackoverflow.com/questions/2078978/functional-
> programming-vs-object-oriented-programming>,
> > we can use method referencing
> > <https://docs.oracle.com/javase/tutorial/java/javaOO/
> methodreferences.html>,
> > usage of aggregate operations, and it will help developers to write
> > memory efficient code.
> >
> >
> > *3.) Using Type Inference*
> > Java uses type inference so to make code lightweight I've updated
> > code constructs as shown in the example for more on this refer this
> article
> > <https://docs.oracle.com/javase/tutorial/java/generics/
> genTypeInference.htmlv>
> > .
> >
> > -        Map<String, ? extends Object> systemProps =
> > UtilGenerics.<String, Object> checkMap(System.getProperties());
> > +        Map<String, ?> systemProps =
> > UtilGenerics.checkMap(System.getProperties());
> >
> >
> > *4.) Use of single quote for character*
> > There is a significant usage of <"Single Character"> in the codebase for
> > example:
> >
> > -            throw new GenericConfigException("Error opening file at
> > location [" + fileUrl.toExternalForm() + "]", e);
> > +            throw new GenericConfigException("Error opening file at
> > location [" + fileUrl.toExternalForm() + ']', e);
> >
> >
> > "]" is comparativlelly slower then ']' Java internally uses Flyweight
> > Design pattern to create String literals so for every call it will not
> > create a new Object and used an existing one this will improve
> > performace to some extend an study can be seen on this
> > <https://stackoverflow.com/questions/24859500/
> concatenate-char-literal-x-vs-single-char-string-literal-x>
> > page.
> >
> >
> > *5.) Updated Variable Declaration*
> >
> > Lastly some of the variable declaration is updated this doesn't create
> > a huge difference but helps JVM at the from implicit conversion.
> >
> > -        private long cumulativeEvents = 0;
> > +        private long cumulativeEvents = 0L;
> >
> >
> > Based on above findings, I have done some code improvement and
> > provided following patches. *And need community help for reviewing
> > these changes.*
> > Kindly provide any improvents or suggestion you have in mind :)
> >
> >
> >    1. [OFBIZ-10344] Refactoring Variable Scope for
> > org.apache.ofbiz.base package
> > <https://issues.apache.org/jira/browse/OFBIZ-10344>
> >    2. [OFBIZ-10345] Refactoring Variable Scope for
> > org.apache.ofbiz.catalina.container
> > <https://issues.apache.org/jira/browse/OFBIZ-10345>
> >    3. [OFBIZ-10346] Refactoring Variable Scope for
> > org.apache.ofbiz.common
> > <https://issues.apache.org/jira/browse/OFBIZ-10346>
> >    4. [OFBIZ-10347] Refactoring Variable Scope for
> > org.apache.ofbiz.datafile
> > <https://issues.apache.org/jira/browse/OFBIZ-10347>
> >    5. [OFBIZ-10348] Refactoring Variable Scope for
> > org.apache.ofbiz.entity
> > <https://issues.apache.org/jira/browse/OFBIZ-10348>
> >
> >
> > P.S. Apart from this I am also working on performance matrix and will
> share
> > it soon.
> >
> >
> > Thanks & Regards,
> >
> > --
> > *Pradhan Yash Sharma*
> >
> >
> > On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma <
> [hidden email]
> >> wrote:
> >
> >> Thank you for the feedback I've created a Jira ticket OFBIZ-10343
> >> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add
> >> patches for the same for your review.
> >>
> >> Thanks & Regards,
> >> --
> >> *Pradhan Yash Sharma*
> >>
> >> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <
> >> [hidden email]> wrote:
> >>
> >>> Hello Pradhan,
> >>>
> >>> Refactoring is exactly what we need and is a welcomed activity. I
> >>> think we should, however, try to avoid "big ideas" across the entire
> >>> code base. The subject of your message is the reason why I say that.
> >>>
> >>> So, if you want to start refactoring, I suggest to start with one
> >>> piece of code, study it careful, issue a JIRA, and provide a patch.
> >>> This should be focused similar to your notes on UtilCache.
> >>>
> >>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
> >>> <[hidden email]> wrote:
> >>> > Hello,
> >>> >
> >>> > While I was working on UtilCache.java file came across some
> >>> improvements,
> >>> > they are as follows:
> >>> >
> >>> > 1) Method and Variable access modifiers can be narrowed down to
> private
> >>> > access modifier.
> >>> >
> >>> > 2) Then AtomicLong can be given value 0L instead of 0.
> >>> >
> >>> > 3) Some Variables is used in both synchronized and unsynchronized
> >>> blocks,
> >>> > so they can be declared final. eg,
> >>> >
> >>> >
> >>> >
> >>> > *protected AtomicLong hitCount = new AtomicLong(0);
> >>> private
> >>> > final AtomicLong hitCount = new AtomicLong(0L);*
> >>> > One variable was able to get most of my attention is
> >>> >
> >>> > *                protected ConcurrentMap<Object, CacheLine<V>>
> >>> memoryTable
> >>> > = null;*
> >>> >
> >>> > This is used in synchronized and unsynchronized blocks, this Object
> can
> >>> be
> >>> > converted into ThreadLocal or AtomicReference but it would require
> >>> changes
> >>> > in the current implementation as well.
> >>> >
> >>> > Lastly, there is extensive use of for loops for iteration we can use
> >>> Java 8
> >>> > Streams, Collector, and other functions to leverage implicit looping
> >>> > mechanism.
> >>> >
> >>> >
> >>> > --
> >>> > Pradhan Yash Sharma
> >>>
> >>
> >>
>

[Yash Sharma]

*
I am not pen downing things, but yea I am really full on high energy to
work on these front.


Thanks & Regards,
--
*Pradhan Yash Sharma*

On Mon, Apr 30, 2018 at 11:37 AM, Yash Sharma <[hidden email]
> wrote:

> Hello,
>
> Thank you for the response, I was wondering about the volume of
> refactoring we can do at each component, so let's apply Divide and Conquer
> approach for each component upgrading work.
>
> I can see a few patterns for the update which I've listed down in my
> previous mail. We can pick any piece of code and apply focused refactoring
> on each component, and then we can do it with others as well when we are
> through with one. It would be a great help if you could suggest a sequence
> to do so for example :
> *I --> 4.) Use of single quote for a character *(this is a
> straightforward work and the easiest one :) ).
> *II --> **3.) Using Type Inference *(We can pick this as it will never
> impact any working code).
> III --> *5.) Updated Variable Declaration.*
> *IV --> **1.) **Downsize Accessibility Scope *(If tested this is also a
> not a big deal).
> *V --> **2.) Using **Lambda Expressions *(This can impact on working
> hence put at last)
>
> This is what I can think :) Please mentor me on this and suggest any
> better action plan we can opt for.
>
> I am very much excited to work on and implement some really cool things
> that Java Ecosystem can offer us like Functional Programming, Jigsaw
> <http://openjdk.java.net/projects/jigsaw/>, or Local Variable Type
> Inference
> <https://developer.oracle.com/java/jdk-10-local-variable-type-inference>,
> I am not penning dowing
>
>
> Thanks & Regards,
> --
> *Pradhan Yash Sharma*
>
> On Sat, Apr 28, 2018 at 6:59 PM, Taher Alkhateeb <
> [hidden email]> wrote:
>
>> Hello Yash,
>>
>> Thank you for your work on this so far. It's great to see people
>> focusing on refactoring, which I think should probably be the top
>> priority for all of us.
>>
>> I will review the JIRAs some more over the coming days, but I have a
>> concern that some of the patches are very large.
>>
>> We had many discussions in the past about focused refactoring vs.
>> general trends. Focused refactoring means you go after a specific
>> piece of code like a class or group of related classes / artifacts and
>> fixing them. General trends, on the other hand, means that you
>> identify a certain pattern and then making a sweeping change across
>> the entire code base.
>>
>> General trends refactorings can be very dangerous, because you are
>> running after a "trend" not isolating a specific piece of code and
>> fixing it.
>>
>> So my recommendation, especially for the bigger patches that you have,
>> is to redesign / refactor so that it is a topic-based, not
>> trend-based. We need to make these commits in isolated bite-sized
>> chunks that focus on a specific area instead of a specific trend (make
>> public to private, add try-with-resources, or whatever else)
>>
>> Cheers,
>> Taher
>>
>> On Thu, Apr 19, 2018 at 3:24 PM, Yash Sharma
>> <[hidden email]> wrote:
>> > Hi Devs,
>> >
>> > Here is the detailed information about the things I am working on for
>> > performance optimization in our OFBiz code.
>> >
>> > *1.) Downsize Accessibility Scope*
>> > I've tried to downsize accessibility scope of classes, interfaces,
>> abstract
>> > class, declared member variables, enumerations, methods, and
>> constructors
>> > to as minimum as possible as per OFBIz current implementation, still
>> there
>> > is a lot of scope for improvement but it would require changes at the
>> > granular level. I've used this
>> > <https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html>
>> as
>> > my reference point. example:
>> >
>> > -    public void noteKeyRemoval(UtilCache<K, V> cache, K key, V
>> oldValue);
>> > +    void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
>> >
>> > Limiting the scope of the method from public modifier to package level.
>> >
>> > *2.) Using Lambda Expressions*
>> > Then tried to use lambda expressions on simple functional work to
>> leverage
>> > implicit type of coding an example:
>> >
>> > -                Map<String, String> initParameters = new
>> LinkedHashMap<>();
>> > -                for (Element e : initParamList) {
>> > -                    initParameters.put(e.getAttribute("name"),
>> > e.getAttribute("value"));
>> > -                }
>> > +                Map<String, String> initParameters =
>> > initParamList.stream().collect(Collectors.toMap(e ->
>> > e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b,
>> > LinkedHashMap::new));
>> >
>> >
>> > Some of the key benefits of using lambdas will introduce Functional
>> > style over Imperative style
>> > <https://stackoverflow.com/questions/2078978/functional-prog
>> ramming-vs-object-oriented-programming>,
>> > we can use method referencing
>> > <https://docs.oracle.com/javase/tutorial/java/javaOO/methodr
>> eferences.html>,
>> > usage of aggregate operations, and it will help developers to write
>> > memory efficient code.
>> >
>> >
>> > *3.) Using Type Inference*
>> > Java uses type inference so to make code lightweight I've updated
>> > code constructs as shown in the example for more on this refer this
>> article
>> > <https://docs.oracle.com/javase/tutorial/java/generics/genTy
>> peInference.htmlv>
>> > .
>> >
>> > -        Map<String, ? extends Object> systemProps =
>> > UtilGenerics.<String, Object> checkMap(System.getProperties());
>> > +        Map<String, ?> systemProps =
>> > UtilGenerics.checkMap(System.getProperties());
>> >
>> >
>> > *4.) Use of single quote for character*
>> > There is a significant usage of <"Single Character"> in the codebase for
>> > example:
>> >
>> > -            throw new GenericConfigException("Error opening file at
>> > location [" + fileUrl.toExternalForm() + "]", e);
>> > +            throw new GenericConfigException("Error opening file at
>> > location [" + fileUrl.toExternalForm() + ']', e);
>> >
>> >
>> > "]" is comparativlelly slower then ']' Java internally uses Flyweight
>> > Design pattern to create String literals so for every call it will not
>> > create a new Object and used an existing one this will improve
>> > performace to some extend an study can be seen on this
>> > <https://stackoverflow.com/questions/24859500/concatenate-
>> char-literal-x-vs-single-char-string-literal-x>
>> > page.
>> >
>> >
>> > *5.) Updated Variable Declaration*
>> >
>> > Lastly some of the variable declaration is updated this doesn't create
>> > a huge difference but helps JVM at the from implicit conversion.
>> >
>> > -        private long cumulativeEvents = 0;
>> > +        private long cumulativeEvents = 0L;
>> >
>> >
>> > Based on above findings, I have done some code improvement and
>> > provided following patches. *And need community help for reviewing
>> > these changes.*
>> > Kindly provide any improvents or suggestion you have in mind :)
>> >
>> >
>> >    1. [OFBIZ-10344] Refactoring Variable Scope for
>> > org.apache.ofbiz.base package
>> > <https://issues.apache.org/jira/browse/OFBIZ-10344>
>> >    2. [OFBIZ-10345] Refactoring Variable Scope for
>> > org.apache.ofbiz.catalina.container
>> > <https://issues.apache.org/jira/browse/OFBIZ-10345>
>> >    3. [OFBIZ-10346] Refactoring Variable Scope for
>> > org.apache.ofbiz.common
>> > <https://issues.apache.org/jira/browse/OFBIZ-10346>
>> >    4. [OFBIZ-10347] Refactoring Variable Scope for
>> > org.apache.ofbiz.datafile
>> > <https://issues.apache.org/jira/browse/OFBIZ-10347>
>> >    5. [OFBIZ-10348] Refactoring Variable Scope for
>> > org.apache.ofbiz.entity
>> > <https://issues.apache.org/jira/browse/OFBIZ-10348>
>> >
>> >
>> > P.S. Apart from this I am also working on performance matrix and will
>> share
>> > it soon.
>> >
>> >
>> > Thanks & Regards,
>> >
>> > --
>> > *Pradhan Yash Sharma*
>> >
>> >
>> > On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma <
>> [hidden email]
>> >> wrote:
>> >
>> >> Thank you for the feedback I've created a Jira ticket OFBIZ-10343
>> >> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add
>> >> patches for the same for your review.
>> >>
>> >> Thanks & Regards,
>> >> --
>> >> *Pradhan Yash Sharma*
>> >>
>> >> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <
>> >> [hidden email]> wrote:
>> >>
>> >>> Hello Pradhan,
>> >>>
>> >>> Refactoring is exactly what we need and is a welcomed activity. I
>> >>> think we should, however, try to avoid "big ideas" across the entire
>> >>> code base. The subject of your message is the reason why I say that.
>> >>>
>> >>> So, if you want to start refactoring, I suggest to start with one
>> >>> piece of code, study it careful, issue a JIRA, and provide a patch.
>> >>> This should be focused similar to your notes on UtilCache.
>> >>>
>> >>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
>> >>> <[hidden email]> wrote:
>> >>> > Hello,
>> >>> >
>> >>> > While I was working on UtilCache.java file came across some
>> >>> improvements,
>> >>> > they are as follows:
>> >>> >
>> >>> > 1) Method and Variable access modifiers can be narrowed down to
>> private
>> >>> > access modifier.
>> >>> >
>> >>> > 2) Then AtomicLong can be given value 0L instead of 0.
>> >>> >
>> >>> > 3) Some Variables is used in both synchronized and unsynchronized
>> >>> blocks,
>> >>> > so they can be declared final. eg,
>> >>> >
>> >>> >
>> >>> >
>> >>> > *protected AtomicLong hitCount = new AtomicLong(0);
>> >>> private
>> >>> > final AtomicLong hitCount = new AtomicLong(0L);*
>> >>> > One variable was able to get most of my attention is
>> >>> >
>> >>> > *                protected ConcurrentMap<Object, CacheLine<V>>
>> >>> memoryTable
>> >>> > = null;*
>> >>> >
>> >>> > This is used in synchronized and unsynchronized blocks, this Object
>> can
>> >>> be
>> >>> > converted into ThreadLocal or AtomicReference but it would require
>> >>> changes
>> >>> > in the current implementation as well.
>> >>> >
>> >>> > Lastly, there is extensive use of for loops for iteration we can use
>> >>> Java 8
>> >>> > Streams, Collector, and other functions to leverage implicit looping
>> >>> > mechanism.
>> >>> >
>> >>> >
>> >>> > --
>> >>> > Pradhan Yash Sharma
>> >>>
>> >>
>> >>
>>
>
>

[taher]

Hello Yash,

So you are proving my point with your feedback. What do I mean by that?

You are focusing on things like the below and then you are submitting
a patch accordingly.
- use lambda expressions
- Type inference
- Using quotes
- whatever else pattern you seek

My recommendation instead is to rework this into:
- I want to refactor this class, and make sure all the code is cleaned up.
- I want to make sure all the methods in this class have a correct
simple signature and I need to make sure everything else works
- These three classes over there are responsible for initiating X, but
they are too complex and have many flaws, we need to set public fields
to private and do A B C ...

The difference between the two above approaches is subtle but very
important. In the first case you are looking at a pattern and you want
to apply it everywhere. The problem is that with this mentality you
are not looking deeply into the code because you are focused on the
pattern. In the second case, you are in fact isolating a single class
or group of related classes and refactoring them very carefully, you
don't care about patterns, you just want to improve the code.

My recommendation is to change the strategy such that your refactoring
one-piece-at-a-time, not one-pattern-at-a-time. I hope this clears up
my perspective

On Mon, Apr 30, 2018 at 9:21 AM, Yash Sharma
<[hidden email]> wrote:

> *
> I am not pen downing things, but yea I am really full on high energy to
> work on these front.
>
>
> Thanks & Regards,
> --
> *Pradhan Yash Sharma*
>
> On Mon, Apr 30, 2018 at 11:37 AM, Yash Sharma <[hidden email]
>> wrote:
>
>> Hello,
>>
>> Thank you for the response, I was wondering about the volume of
>> refactoring we can do at each component, so let's apply Divide and Conquer
>> approach for each component upgrading work.
>>
>> I can see a few patterns for the update which I've listed down in my
>> previous mail. We can pick any piece of code and apply focused refactoring
>> on each component, and then we can do it with others as well when we are
>> through with one. It would be a great help if you could suggest a sequence
>> to do so for example :
>> *I --> 4.) Use of single quote for a character *(this is a
>> straightforward work and the easiest one :) ).
>> *II --> **3.) Using Type Inference *(We can pick this as it will never
>> impact any working code).
>> III --> *5.) Updated Variable Declaration.*
>> *IV --> **1.) **Downsize Accessibility Scope *(If tested this is also a
>> not a big deal).
>> *V --> **2.) Using **Lambda Expressions *(This can impact on working
>> hence put at last)
>>
>> This is what I can think :) Please mentor me on this and suggest any
>> better action plan we can opt for.
>>
>> I am very much excited to work on and implement some really cool things
>> that Java Ecosystem can offer us like Functional Programming, Jigsaw
>> <http://openjdk.java.net/projects/jigsaw/>, or Local Variable Type
>> Inference
>> <https://developer.oracle.com/java/jdk-10-local-variable-type-inference>,
>> I am not penning dowing
>>
>>
>> Thanks & Regards,
>> --
>> *Pradhan Yash Sharma*
>>
>> On Sat, Apr 28, 2018 at 6:59 PM, Taher Alkhateeb <
>> [hidden email]> wrote:
>>
>>> Hello Yash,
>>>
>>> Thank you for your work on this so far. It's great to see people
>>> focusing on refactoring, which I think should probably be the top
>>> priority for all of us.
>>>
>>> I will review the JIRAs some more over the coming days, but I have a
>>> concern that some of the patches are very large.
>>>
>>> We had many discussions in the past about focused refactoring vs.
>>> general trends. Focused refactoring means you go after a specific
>>> piece of code like a class or group of related classes / artifacts and
>>> fixing them. General trends, on the other hand, means that you
>>> identify a certain pattern and then making a sweeping change across
>>> the entire code base.
>>>
>>> General trends refactorings can be very dangerous, because you are
>>> running after a "trend" not isolating a specific piece of code and
>>> fixing it.
>>>
>>> So my recommendation, especially for the bigger patches that you have,
>>> is to redesign / refactor so that it is a topic-based, not
>>> trend-based. We need to make these commits in isolated bite-sized
>>> chunks that focus on a specific area instead of a specific trend (make
>>> public to private, add try-with-resources, or whatever else)
>>>
>>> Cheers,
>>> Taher
>>>
>>> On Thu, Apr 19, 2018 at 3:24 PM, Yash Sharma
>>> <[hidden email]> wrote:
>>> > Hi Devs,
>>> >
>>> > Here is the detailed information about the things I am working on for
>>> > performance optimization in our OFBiz code.
>>> >
>>> > *1.) Downsize Accessibility Scope*
>>> > I've tried to downsize accessibility scope of classes, interfaces,
>>> abstract
>>> > class, declared member variables, enumerations, methods, and
>>> constructors
>>> > to as minimum as possible as per OFBIz current implementation, still
>>> there
>>> > is a lot of scope for improvement but it would require changes at the
>>> > granular level. I've used this
>>> > <https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html>
>>> as
>>> > my reference point. example:
>>> >
>>> > -    public void noteKeyRemoval(UtilCache<K, V> cache, K key, V
>>> oldValue);
>>> > +    void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
>>> >
>>> > Limiting the scope of the method from public modifier to package level.
>>> >
>>> > *2.) Using Lambda Expressions*
>>> > Then tried to use lambda expressions on simple functional work to
>>> leverage
>>> > implicit type of coding an example:
>>> >
>>> > -                Map<String, String> initParameters = new
>>> LinkedHashMap<>();
>>> > -                for (Element e : initParamList) {
>>> > -                    initParameters.put(e.getAttribute("name"),
>>> > e.getAttribute("value"));
>>> > -                }
>>> > +                Map<String, String> initParameters =
>>> > initParamList.stream().collect(Collectors.toMap(e ->
>>> > e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b,
>>> > LinkedHashMap::new));
>>> >
>>> >
>>> > Some of the key benefits of using lambdas will introduce Functional
>>> > style over Imperative style
>>> > <https://stackoverflow.com/questions/2078978/functional-prog
>>> ramming-vs-object-oriented-programming>,
>>> > we can use method referencing
>>> > <https://docs.oracle.com/javase/tutorial/java/javaOO/methodr
>>> eferences.html>,
>>> > usage of aggregate operations, and it will help developers to write
>>> > memory efficient code.
>>> >
>>> >
>>> > *3.) Using Type Inference*
>>> > Java uses type inference so to make code lightweight I've updated
>>> > code constructs as shown in the example for more on this refer this
>>> article
>>> > <https://docs.oracle.com/javase/tutorial/java/generics/genTy
>>> peInference.htmlv>
>>> > .
>>> >
>>> > -        Map<String, ? extends Object> systemProps =
>>> > UtilGenerics.<String, Object> checkMap(System.getProperties());
>>> > +        Map<String, ?> systemProps =
>>> > UtilGenerics.checkMap(System.getProperties());
>>> >
>>> >
>>> > *4.) Use of single quote for character*
>>> > There is a significant usage of <"Single Character"> in the codebase for
>>> > example:
>>> >
>>> > -            throw new GenericConfigException("Error opening file at
>>> > location [" + fileUrl.toExternalForm() + "]", e);
>>> > +            throw new GenericConfigException("Error opening file at
>>> > location [" + fileUrl.toExternalForm() + ']', e);
>>> >
>>> >
>>> > "]" is comparativlelly slower then ']' Java internally uses Flyweight
>>> > Design pattern to create String literals so for every call it will not
>>> > create a new Object and used an existing one this will improve
>>> > performace to some extend an study can be seen on this
>>> > <https://stackoverflow.com/questions/24859500/concatenate-
>>> char-literal-x-vs-single-char-string-literal-x>
>>> > page.
>>> >
>>> >
>>> > *5.) Updated Variable Declaration*
>>> >
>>> > Lastly some of the variable declaration is updated this doesn't create
>>> > a huge difference but helps JVM at the from implicit conversion.
>>> >
>>> > -        private long cumulativeEvents = 0;
>>> > +        private long cumulativeEvents = 0L;
>>> >
>>> >
>>> > Based on above findings, I have done some code improvement and
>>> > provided following patches. *And need community help for reviewing
>>> > these changes.*
>>> > Kindly provide any improvents or suggestion you have in mind :)
>>> >
>>> >
>>> >    1. [OFBIZ-10344] Refactoring Variable Scope for
>>> > org.apache.ofbiz.base package
>>> > <https://issues.apache.org/jira/browse/OFBIZ-10344>
>>> >    2. [OFBIZ-10345] Refactoring Variable Scope for
>>> > org.apache.ofbiz.catalina.container
>>> > <https://issues.apache.org/jira/browse/OFBIZ-10345>
>>> >    3. [OFBIZ-10346] Refactoring Variable Scope for
>>> > org.apache.ofbiz.common
>>> > <https://issues.apache.org/jira/browse/OFBIZ-10346>
>>> >    4. [OFBIZ-10347] Refactoring Variable Scope for
>>> > org.apache.ofbiz.datafile
>>> > <https://issues.apache.org/jira/browse/OFBIZ-10347>
>>> >    5. [OFBIZ-10348] Refactoring Variable Scope for
>>> > org.apache.ofbiz.entity
>>> > <https://issues.apache.org/jira/browse/OFBIZ-10348>
>>> >
>>> >
>>> > P.S. Apart from this I am also working on performance matrix and will
>>> share
>>> > it soon.
>>> >
>>> >
>>> > Thanks & Regards,
>>> >
>>> > --
>>> > *Pradhan Yash Sharma*
>>> >
>>> >
>>> > On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma <
>>> [hidden email]
>>> >> wrote:
>>> >
>>> >> Thank you for the feedback I've created a Jira ticket OFBIZ-10343
>>> >> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add
>>> >> patches for the same for your review.
>>> >>
>>> >> Thanks & Regards,
>>> >> --
>>> >> *Pradhan Yash Sharma*
>>> >>
>>> >> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <
>>> >> [hidden email]> wrote:
>>> >>
>>> >>> Hello Pradhan,
>>> >>>
>>> >>> Refactoring is exactly what we need and is a welcomed activity. I
>>> >>> think we should, however, try to avoid "big ideas" across the entire
>>> >>> code base. The subject of your message is the reason why I say that.
>>> >>>
>>> >>> So, if you want to start refactoring, I suggest to start with one
>>> >>> piece of code, study it careful, issue a JIRA, and provide a patch.
>>> >>> This should be focused similar to your notes on UtilCache.
>>> >>>
>>> >>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
>>> >>> <[hidden email]> wrote:
>>> >>> > Hello,
>>> >>> >
>>> >>> > While I was working on UtilCache.java file came across some
>>> >>> improvements,
>>> >>> > they are as follows:
>>> >>> >
>>> >>> > 1) Method and Variable access modifiers can be narrowed down to
>>> private
>>> >>> > access modifier.
>>> >>> >
>>> >>> > 2) Then AtomicLong can be given value 0L instead of 0.
>>> >>> >
>>> >>> > 3) Some Variables is used in both synchronized and unsynchronized
>>> >>> blocks,
>>> >>> > so they can be declared final. eg,
>>> >>> >
>>> >>> >
>>> >>> >
>>> >>> > *protected AtomicLong hitCount = new AtomicLong(0);
>>> >>> private
>>> >>> > final AtomicLong hitCount = new AtomicLong(0L);*
>>> >>> > One variable was able to get most of my attention is
>>> >>> >
>>> >>> > *                protected ConcurrentMap<Object, CacheLine<V>>
>>> >>> memoryTable
>>> >>> > = null;*
>>> >>> >
>>> >>> > This is used in synchronized and unsynchronized blocks, this Object
>>> can
>>> >>> be
>>> >>> > converted into ThreadLocal or AtomicReference but it would require
>>> >>> changes
>>> >>> > in the current implementation as well.
>>> >>> >
>>> >>> > Lastly, there is extensive use of for loops for iteration we can use
>>> >>> Java 8
>>> >>> > Streams, Collector, and other functions to leverage implicit looping
>>> >>> > mechanism.
>>> >>> >
>>> >>> >
>>> >>> > --
>>> >>> > Pradhan Yash Sharma
>>> >>>
>>> >>
>>> >>
>>>
>>
>>

[Yash Sharma]

Hello Taher,

Yes certainly you cleared my doubts, and Yes I am confirming your point
with the feedback.

What I get after this entire discussion is, I was concentrating on
Refactoring and you are emphasizing on Re-Engineering (it will not be an
OFBiz Update with new tricks it will be an OFBiz Upgrade with workflow and
may involve some new tricks).

If this is what you meant, then I observe the pathway will be challenging
but, I guess everyone will understand it will be pleasant to do so.

Looking forward to your approval. It would be a great help if you could
propose an action plan.

Thanks & Regards,
--
*Pradhan Yash Sharma*


On Tue, May 1, 2018 at 3:59 PM Taher Alkhateeb <[hidden email]>
wrote:

> Hello Yash,
>
> So you are proving my point with your feedback. What do I mean by that?
>
> You are focusing on things like the below and then you are submitting
> a patch accordingly.
> - use lambda expressions
> - Type inference
> - Using quotes
> - whatever else pattern you seek
>
> My recommendation instead is to rework this into:
> - I want to refactor this class, and make sure all the code is cleaned up.
> - I want to make sure all the methods in this class have a correct
> simple signature and I need to make sure everything else works
> - These three classes over there are responsible for initiating X, but
> they are too complex and have many flaws, we need to set public fields
> to private and do A B C ...
>
> The difference between the two above approaches is subtle but very
> important. In the first case you are looking at a pattern and you want
> to apply it everywhere. The problem is that with this mentality you
> are not looking deeply into the code because you are focused on the
> pattern. In the second case, you are in fact isolating a single class
> or group of related classes and refactoring them very carefully, you
> don't care about patterns, you just want to improve the code.
>
> My recommendation is to change the strategy such that your refactoring
> one-piece-at-a-time, not one-pattern-at-a-time. I hope this clears up
> my perspective
>
> On Mon, Apr 30, 2018 at 9:21 AM, Yash Sharma
> <[hidden email]> wrote:
> > *
> > I am not pen downing things, but yea I am really full on high energy to
> > work on these front.
> >
> >
> > Thanks & Regards,
> > --
> > *Pradhan Yash Sharma*
> >
> > On Mon, Apr 30, 2018 at 11:37 AM, Yash Sharma <
> [hidden email]
> >> wrote:
> >
> >> Hello,
> >>
> >> Thank you for the response, I was wondering about the volume of
> >> refactoring we can do at each component, so let's apply Divide and
> Conquer
> >> approach for each component upgrading work.
> >>
> >> I can see a few patterns for the update which I've listed down in my
> >> previous mail. We can pick any piece of code and apply focused
> refactoring
> >> on each component, and then we can do it with others as well when we are
> >> through with one. It would be a great help if you could suggest a
> sequence
> >> to do so for example :
> >> *I --> 4.) Use of single quote for a character *(this is a
> >> straightforward work and the easiest one :) ).
> >> *II --> **3.) Using Type Inference *(We can pick this as it will never
> >> impact any working code).
> >> III --> *5.) Updated Variable Declaration.*
> >> *IV --> **1.) **Downsize Accessibility Scope *(If tested this is also a
> >> not a big deal).
> >> *V --> **2.) Using **Lambda Expressions *(This can impact on working
> >> hence put at last)
> >>
> >> This is what I can think :) Please mentor me on this and suggest any
> >> better action plan we can opt for.
> >>
> >> I am very much excited to work on and implement some really cool things
> >> that Java Ecosystem can offer us like Functional Programming, Jigsaw
> >> <http://openjdk.java.net/projects/jigsaw/>, or Local Variable Type
> >> Inference
> >> <https://developer.oracle.com/java/jdk-10-local-variable-type-inference
> >,
> >> I am not penning dowing
> >>
> >>
> >> Thanks & Regards,
> >> --
> >> *Pradhan Yash Sharma*
> >>
> >> On Sat, Apr 28, 2018 at 6:59 PM, Taher Alkhateeb <
> >> [hidden email]> wrote:
> >>
> >>> Hello Yash,
> >>>
> >>> Thank you for your work on this so far. It's great to see people
> >>> focusing on refactoring, which I think should probably be the top
> >>> priority for all of us.
> >>>
> >>> I will review the JIRAs some more over the coming days, but I have a
> >>> concern that some of the patches are very large.
> >>>
> >>> We had many discussions in the past about focused refactoring vs.
> >>> general trends. Focused refactoring means you go after a specific
> >>> piece of code like a class or group of related classes / artifacts and
> >>> fixing them. General trends, on the other hand, means that you
> >>> identify a certain pattern and then making a sweeping change across
> >>> the entire code base.
> >>>
> >>> General trends refactorings can be very dangerous, because you are
> >>> running after a "trend" not isolating a specific piece of code and
> >>> fixing it.
> >>>
> >>> So my recommendation, especially for the bigger patches that you have,
> >>> is to redesign / refactor so that it is a topic-based, not
> >>> trend-based. We need to make these commits in isolated bite-sized
> >>> chunks that focus on a specific area instead of a specific trend (make
> >>> public to private, add try-with-resources, or whatever else)
> >>>
> >>> Cheers,
> >>> Taher
> >>>
> >>> On Thu, Apr 19, 2018 at 3:24 PM, Yash Sharma
> >>> <[hidden email]> wrote:
> >>> > Hi Devs,
> >>> >
> >>> > Here is the detailed information about the things I am working on for
> >>> > performance optimization in our OFBiz code.
> >>> >
> >>> > *1.) Downsize Accessibility Scope*
> >>> > I've tried to downsize accessibility scope of classes, interfaces,
> >>> abstract
> >>> > class, declared member variables, enumerations, methods, and
> >>> constructors
> >>> > to as minimum as possible as per OFBIz current implementation, still
> >>> there
> >>> > is a lot of scope for improvement but it would require changes at the
> >>> > granular level. I've used this
> >>> > <
> https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html>
> >>> as
> >>> > my reference point. example:
> >>> >
> >>> > -    public void noteKeyRemoval(UtilCache<K, V> cache, K key, V
> >>> oldValue);
> >>> > +    void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
> >>> >
> >>> > Limiting the scope of the method from public modifier to package
> level.
> >>> >
> >>> > *2.) Using Lambda Expressions*
> >>> > Then tried to use lambda expressions on simple functional work to
> >>> leverage
> >>> > implicit type of coding an example:
> >>> >
> >>> > -                Map<String, String> initParameters = new
> >>> LinkedHashMap<>();
> >>> > -                for (Element e : initParamList) {
> >>> > -                    initParameters.put(e.getAttribute("name"),
> >>> > e.getAttribute("value"));
> >>> > -                }
> >>> > +                Map<String, String> initParameters =
> >>> > initParamList.stream().collect(Collectors.toMap(e ->
> >>> > e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b,
> >>> > LinkedHashMap::new));
> >>> >
> >>> >
> >>> > Some of the key benefits of using lambdas will introduce Functional
> >>> > style over Imperative style
> >>> > <https://stackoverflow.com/questions/2078978/functional-prog
> >>> ramming-vs-object-oriented-programming>,
> >>> > we can use method referencing
> >>> > <https://docs.oracle.com/javase/tutorial/java/javaOO/methodr
> >>> eferences.html>,
> >>> > usage of aggregate operations, and it will help developers to write
> >>> > memory efficient code.
> >>> >
> >>> >
> >>> > *3.) Using Type Inference*
> >>> > Java uses type inference so to make code lightweight I've updated
> >>> > code constructs as shown in the example for more on this refer this
> >>> article
> >>> > <https://docs.oracle.com/javase/tutorial/java/generics/genTy
> >>> peInference.htmlv>
> >>> > .
> >>> >
> >>> > -        Map<String, ? extends Object> systemProps =
> >>> > UtilGenerics.<String, Object> checkMap(System.getProperties());
> >>> > +        Map<String, ?> systemProps =
> >>> > UtilGenerics.checkMap(System.getProperties());
> >>> >
> >>> >
> >>> > *4.) Use of single quote for character*
> >>> > There is a significant usage of <"Single Character"> in the codebase
> for
> >>> > example:
> >>> >
> >>> > -            throw new GenericConfigException("Error opening file at
> >>> > location [" + fileUrl.toExternalForm() + "]", e);
> >>> > +            throw new GenericConfigException("Error opening file at
> >>> > location [" + fileUrl.toExternalForm() + ']', e);
> >>> >
> >>> >
> >>> > "]" is comparativlelly slower then ']' Java internally uses Flyweight
> >>> > Design pattern to create String literals so for every call it will
> not
> >>> > create a new Object and used an existing one this will improve
> >>> > performace to some extend an study can be seen on this
> >>> > <https://stackoverflow.com/questions/24859500/concatenate-
> >>> char-literal-x-vs-single-char-string-literal-x>
> >>> > page.
> >>> >
> >>> >
> >>> > *5.) Updated Variable Declaration*
> >>> >
> >>> > Lastly some of the variable declaration is updated this doesn't
> create
> >>> > a huge difference but helps JVM at the from implicit conversion.
> >>> >
> >>> > -        private long cumulativeEvents = 0;
> >>> > +        private long cumulativeEvents = 0L;
> >>> >
> >>> >
> >>> > Based on above findings, I have done some code improvement and
> >>> > provided following patches. *And need community help for reviewing
> >>> > these changes.*
> >>> > Kindly provide any improvents or suggestion you have in mind :)
> >>> >
> >>> >
> >>> >    1. [OFBIZ-10344] Refactoring Variable Scope for
> >>> > org.apache.ofbiz.base package
> >>> > <https://issues.apache.org/jira/browse/OFBIZ-10344>
> >>> >    2. [OFBIZ-10345] Refactoring Variable Scope for
> >>> > org.apache.ofbiz.catalina.container
> >>> > <https://issues.apache.org/jira/browse/OFBIZ-10345>
> >>> >    3. [OFBIZ-10346] Refactoring Variable Scope for
> >>> > org.apache.ofbiz.common
> >>> > <https://issues.apache.org/jira/browse/OFBIZ-10346>
> >>> >    4. [OFBIZ-10347] Refactoring Variable Scope for
> >>> > org.apache.ofbiz.datafile
> >>> > <https://issues.apache.org/jira/browse/OFBIZ-10347>
> >>> >    5. [OFBIZ-10348] Refactoring Variable Scope for
> >>> > org.apache.ofbiz.entity
> >>> > <https://issues.apache.org/jira/browse/OFBIZ-10348>
> >>> >
> >>> >
> >>> > P.S. Apart from this I am also working on performance matrix and will
> >>> share
> >>> > it soon.
> >>> >
> >>> >
> >>> > Thanks & Regards,
> >>> >
> >>> > --
> >>> > *Pradhan Yash Sharma*
> >>> >
> >>> >
> >>> > On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma <
> >>> [hidden email]
> >>> >> wrote:
> >>> >
> >>> >> Thank you for the feedback I've created a Jira ticket OFBIZ-10343
> >>> >> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will add
> >>> >> patches for the same for your review.
> >>> >>
> >>> >> Thanks & Regards,
> >>> >> --
> >>> >> *Pradhan Yash Sharma*
> >>> >>
> >>> >> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <
> >>> >> [hidden email]> wrote:
> >>> >>
> >>> >>> Hello Pradhan,
> >>> >>>
> >>> >>> Refactoring is exactly what we need and is a welcomed activity. I
> >>> >>> think we should, however, try to avoid "big ideas" across the
> entire
> >>> >>> code base. The subject of your message is the reason why I say
> that.
> >>> >>>
> >>> >>> So, if you want to start refactoring, I suggest to start with one
> >>> >>> piece of code, study it careful, issue a JIRA, and provide a patch.
> >>> >>> This should be focused similar to your notes on UtilCache.
> >>> >>>
> >>> >>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
> >>> >>> <[hidden email]> wrote:
> >>> >>> > Hello,
> >>> >>> >
> >>> >>> > While I was working on UtilCache.java file came across some
> >>> >>> improvements,
> >>> >>> > they are as follows:
> >>> >>> >
> >>> >>> > 1) Method and Variable access modifiers can be narrowed down to
> >>> private
> >>> >>> > access modifier.
> >>> >>> >
> >>> >>> > 2) Then AtomicLong can be given value 0L instead of 0.
> >>> >>> >
> >>> >>> > 3) Some Variables is used in both synchronized and unsynchronized
> >>> >>> blocks,
> >>> >>> > so they can be declared final. eg,
> >>> >>> >
> >>> >>> >
> >>> >>> >
> >>> >>> > *protected AtomicLong hitCount = new AtomicLong(0);
> >>> >>> private
> >>> >>> > final AtomicLong hitCount = new AtomicLong(0L);*
> >>> >>> > One variable was able to get most of my attention is
> >>> >>> >
> >>> >>> > *                protected ConcurrentMap<Object, CacheLine<V>>
> >>> >>> memoryTable
> >>> >>> > = null;*
> >>> >>> >
> >>> >>> > This is used in synchronized and unsynchronized blocks, this
> Object
> >>> can
> >>> >>> be
> >>> >>> > converted into ThreadLocal or AtomicReference but it would
> require
> >>> >>> changes
> >>> >>> > in the current implementation as well.
> >>> >>> >
> >>> >>> > Lastly, there is extensive use of for loops for iteration we can
> use
> >>> >>> Java 8
> >>> >>> > Streams, Collector, and other functions to leverage implicit
> looping
> >>> >>> > mechanism.
> >>> >>> >
> >>> >>> >
> >>> >>> > --
> >>> >>> > Pradhan Yash Sharma
> >>> >>>
> >>> >>
> >>> >>
> >>>
> >>
> >>
>

[taher]

Hello Yash, Inline ...

On Wed, May 2, 2018, 8:59 AM Yash Sharma <[hidden email]>
wrote:

> Hello Taher,
>
> Yes certainly you cleared my doubts, and Yes I am confirming your point
> with the feedback.
>
> What I get after this entire discussion is, I was concentrating on
> Refactoring and you are emphasizing on Re-Engineering


Actually I think it is still called refactoring because you are changing
code, not system behavior. [1]

(it will not be an
> OFBiz Update with new tricks it will be an OFBiz Upgrade with workflow and
> may involve some new tricks).
>

It will be the same system with the same behavior but with some improved
code


> If this is what you meant, then I observe the pathway will be challenging
> but, I guess everyone will understand it will be pleasant to do so.
>

Exactly! it will be challenging, it _should_ be challenging. Coding is not
something to be taken lightly especially with a massive interconnected
system like OFBiz.


> Looking forward to your approval. It would be a great help if you could
> propose an action plan.


My approval is irrelevant, we are a community and we value consensus and
teamwork and we all play on an equal level here :) I am merely expressing
thoughts that might be helpful.

So what should you do? May I recommend a simple approach? For example,
instead of making a patch for a 100 different classes, make a patch for
only one or few related classes. The smaller the commit size the better
because it will allow proper peer-review. It will also increase the speed
of getting your work committed.

Your enthusiasm and dedication is great, I look forward to working with you
in the coming days.


> Thanks & Regards,
> --
> *Pradhan Yash Sharma*
>
[1] https://en.m.wikipedia.org/wiki/Code_refactoring

>
> On Tue, May 1, 2018 at 3:59 PM Taher Alkhateeb <[hidden email]
> >
> wrote:
>
> > Hello Yash,
> >
> > So you are proving my point with your feedback. What do I mean by that?
> >
> > You are focusing on things like the below and then you are submitting
> > a patch accordingly.
> > - use lambda expressions
> > - Type inference
> > - Using quotes
> > - whatever else pattern you seek
> >
> > My recommendation instead is to rework this into:
> > - I want to refactor this class, and make sure all the code is cleaned
> up.
> > - I want to make sure all the methods in this class have a correct
> > simple signature and I need to make sure everything else works
> > - These three classes over there are responsible for initiating X, but
> > they are too complex and have many flaws, we need to set public fields
> > to private and do A B C ...
> >
> > The difference between the two above approaches is subtle but very
> > important. In the first case you are looking at a pattern and you want
> > to apply it everywhere. The problem is that with this mentality you
> > are not looking deeply into the code because you are focused on the
> > pattern. In the second case, you are in fact isolating a single class
> > or group of related classes and refactoring them very carefully, you
> > don't care about patterns, you just want to improve the code.
> >
> > My recommendation is to change the strategy such that your refactoring
> > one-piece-at-a-time, not one-pattern-at-a-time. I hope this clears up
> > my perspective
> >
> > On Mon, Apr 30, 2018 at 9:21 AM, Yash Sharma
> > <[hidden email]> wrote:
> > > *
> > > I am not pen downing things, but yea I am really full on high energy to
> > > work on these front.
> > >
> > >
> > > Thanks & Regards,
> > > --
> > > *Pradhan Yash Sharma*
> > >
> > > On Mon, Apr 30, 2018 at 11:37 AM, Yash Sharma <
> > [hidden email]
> > >> wrote:
> > >
> > >> Hello,
> > >>
> > >> Thank you for the response, I was wondering about the volume of
> > >> refactoring we can do at each component, so let's apply Divide and
> > Conquer
> > >> approach for each component upgrading work.
> > >>
> > >> I can see a few patterns for the update which I've listed down in my
> > >> previous mail. We can pick any piece of code and apply focused
> > refactoring
> > >> on each component, and then we can do it with others as well when we
> are
> > >> through with one. It would be a great help if you could suggest a
> > sequence
> > >> to do so for example :
> > >> *I --> 4.) Use of single quote for a character *(this is a
> > >> straightforward work and the easiest one :) ).
> > >> *II --> **3.) Using Type Inference *(We can pick this as it will never
> > >> impact any working code).
> > >> III --> *5.) Updated Variable Declaration.*
> > >> *IV --> **1.) **Downsize Accessibility Scope *(If tested this is also
> a
> > >> not a big deal).
> > >> *V --> **2.) Using **Lambda Expressions *(This can impact on working
> > >> hence put at last)
> > >>
> > >> This is what I can think :) Please mentor me on this and suggest any
> > >> better action plan we can opt for.
> > >>
> > >> I am very much excited to work on and implement some really cool
> things
> > >> that Java Ecosystem can offer us like Functional Programming, Jigsaw
> > >> <http://openjdk.java.net/projects/jigsaw/>, or Local Variable Type
> > >> Inference
> > >> <
> https://developer.oracle.com/java/jdk-10-local-variable-type-inference
> > >,
> > >> I am not penning dowing
> > >>
> > >>
> > >> Thanks & Regards,
> > >> --
> > >> *Pradhan Yash Sharma*
> > >>
> > >> On Sat, Apr 28, 2018 at 6:59 PM, Taher Alkhateeb <
> > >> [hidden email]> wrote:
> > >>
> > >>> Hello Yash,
> > >>>
> > >>> Thank you for your work on this so far. It's great to see people
> > >>> focusing on refactoring, which I think should probably be the top
> > >>> priority for all of us.
> > >>>
> > >>> I will review the JIRAs some more over the coming days, but I have a
> > >>> concern that some of the patches are very large.
> > >>>
> > >>> We had many discussions in the past about focused refactoring vs.
> > >>> general trends. Focused refactoring means you go after a specific
> > >>> piece of code like a class or group of related classes / artifacts
> and
> > >>> fixing them. General trends, on the other hand, means that you
> > >>> identify a certain pattern and then making a sweeping change across
> > >>> the entire code base.
> > >>>
> > >>> General trends refactorings can be very dangerous, because you are
> > >>> running after a "trend" not isolating a specific piece of code and
> > >>> fixing it.
> > >>>
> > >>> So my recommendation, especially for the bigger patches that you
> have,
> > >>> is to redesign / refactor so that it is a topic-based, not
> > >>> trend-based. We need to make these commits in isolated bite-sized
> > >>> chunks that focus on a specific area instead of a specific trend
> (make
> > >>> public to private, add try-with-resources, or whatever else)
> > >>>
> > >>> Cheers,
> > >>> Taher
> > >>>
> > >>> On Thu, Apr 19, 2018 at 3:24 PM, Yash Sharma
> > >>> <[hidden email]> wrote:
> > >>> > Hi Devs,
> > >>> >
> > >>> > Here is the detailed information about the things I am working on
> for
> > >>> > performance optimization in our OFBiz code.
> > >>> >
> > >>> > *1.) Downsize Accessibility Scope*
> > >>> > I've tried to downsize accessibility scope of classes, interfaces,
> > >>> abstract
> > >>> > class, declared member variables, enumerations, methods, and
> > >>> constructors
> > >>> > to as minimum as possible as per OFBIz current implementation,
> still
> > >>> there
> > >>> > is a lot of scope for improvement but it would require changes at
> the
> > >>> > granular level. I've used this
> > >>> > <
> > https://docs.oracle.com/javase/tutorial/java/javaOO/accesscontrol.html>
> > >>> as
> > >>> > my reference point. example:
> > >>> >
> > >>> > -    public void noteKeyRemoval(UtilCache<K, V> cache, K key, V
> > >>> oldValue);
> > >>> > +    void noteKeyRemoval(UtilCache<K, V> cache, K key, V oldValue);
> > >>> >
> > >>> > Limiting the scope of the method from public modifier to package
> > level.
> > >>> >
> > >>> > *2.) Using Lambda Expressions*
> > >>> > Then tried to use lambda expressions on simple functional work to
> > >>> leverage
> > >>> > implicit type of coding an example:
> > >>> >
> > >>> > -                Map<String, String> initParameters = new
> > >>> LinkedHashMap<>();
> > >>> > -                for (Element e : initParamList) {
> > >>> > -                    initParameters.put(e.getAttribute("name"),
> > >>> > e.getAttribute("value"));
> > >>> > -                }
> > >>> > +                Map<String, String> initParameters =
> > >>> > initParamList.stream().collect(Collectors.toMap(e ->
> > >>> > e.getAttribute("name"), e -> e.getAttribute("value"), (a, b) -> b,
> > >>> > LinkedHashMap::new));
> > >>> >
> > >>> >
> > >>> > Some of the key benefits of using lambdas will introduce Functional
> > >>> > style over Imperative style
> > >>> > <https://stackoverflow.com/questions/2078978/functional-prog
> > >>> ramming-vs-object-oriented-programming>,
> > >>> > we can use method referencing
> > >>> > <https://docs.oracle.com/javase/tutorial/java/javaOO/methodr
> > >>> eferences.html>,
> > >>> > usage of aggregate operations, and it will help developers to write
> > >>> > memory efficient code.
> > >>> >
> > >>> >
> > >>> > *3.) Using Type Inference*
> > >>> > Java uses type inference so to make code lightweight I've updated
> > >>> > code constructs as shown in the example for more on this refer this
> > >>> article
> > >>> > <https://docs.oracle.com/javase/tutorial/java/generics/genTy
> > >>> peInference.htmlv>
> > >>> > .
> > >>> >
> > >>> > -        Map<String, ? extends Object> systemProps =
> > >>> > UtilGenerics.<String, Object> checkMap(System.getProperties());
> > >>> > +        Map<String, ?> systemProps =
> > >>> > UtilGenerics.checkMap(System.getProperties());
> > >>> >
> > >>> >
> > >>> > *4.) Use of single quote for character*
> > >>> > There is a significant usage of <"Single Character"> in the
> codebase
> > for
> > >>> > example:
> > >>> >
> > >>> > -            throw new GenericConfigException("Error opening file
> at
> > >>> > location [" + fileUrl.toExternalForm() + "]", e);
> > >>> > +            throw new GenericConfigException("Error opening file
> at
> > >>> > location [" + fileUrl.toExternalForm() + ']', e);
> > >>> >
> > >>> >
> > >>> > "]" is comparativlelly slower then ']' Java internally uses
> Flyweight
> > >>> > Design pattern to create String literals so for every call it will
> > not
> > >>> > create a new Object and used an existing one this will improve
> > >>> > performace to some extend an study can be seen on this
> > >>> > <https://stackoverflow.com/questions/24859500/concatenate-
> > >>> char-literal-x-vs-single-char-string-literal-x>
> > >>> > page.
> > >>> >
> > >>> >
> > >>> > *5.) Updated Variable Declaration*
> > >>> >
> > >>> > Lastly some of the variable declaration is updated this doesn't
> > create
> > >>> > a huge difference but helps JVM at the from implicit conversion.
> > >>> >
> > >>> > -        private long cumulativeEvents = 0;
> > >>> > +        private long cumulativeEvents = 0L;
> > >>> >
> > >>> >
> > >>> > Based on above findings, I have done some code improvement and
> > >>> > provided following patches. *And need community help for reviewing
> > >>> > these changes.*
> > >>> > Kindly provide any improvents or suggestion you have in mind :)
> > >>> >
> > >>> >
> > >>> >    1. [OFBIZ-10344] Refactoring Variable Scope for
> > >>> > org.apache.ofbiz.base package
> > >>> > <https://issues.apache.org/jira/browse/OFBIZ-10344>
> > >>> >    2. [OFBIZ-10345] Refactoring Variable Scope for
> > >>> > org.apache.ofbiz.catalina.container
> > >>> > <https://issues.apache.org/jira/browse/OFBIZ-10345>
> > >>> >    3. [OFBIZ-10346] Refactoring Variable Scope for
> > >>> > org.apache.ofbiz.common
> > >>> > <https://issues.apache.org/jira/browse/OFBIZ-10346>
> > >>> >    4. [OFBIZ-10347] Refactoring Variable Scope for
> > >>> > org.apache.ofbiz.datafile
> > >>> > <https://issues.apache.org/jira/browse/OFBIZ-10347>
> > >>> >    5. [OFBIZ-10348] Refactoring Variable Scope for
> > >>> > org.apache.ofbiz.entity
> > >>> > <https://issues.apache.org/jira/browse/OFBIZ-10348>
> > >>> >
> > >>> >
> > >>> > P.S. Apart from this I am also working on performance matrix and
> will
> > >>> share
> > >>> > it soon.
> > >>> >
> > >>> >
> > >>> > Thanks & Regards,
> > >>> >
> > >>> > --
> > >>> > *Pradhan Yash Sharma*
> > >>> >
> > >>> >
> > >>> > On Tue, Apr 17, 2018 at 11:28 AM, Yash Sharma <
> > >>> [hidden email]
> > >>> >> wrote:
> > >>> >
> > >>> >> Thank you for the feedback I've created a Jira ticket OFBIZ-10343
> > >>> >> <https://issues.apache.org/jira/browse/OFBIZ-10343> and I will
> add
> > >>> >> patches for the same for your review.
> > >>> >>
> > >>> >> Thanks & Regards,
> > >>> >> --
> > >>> >> *Pradhan Yash Sharma*
> > >>> >>
> > >>> >> On Fri, Apr 13, 2018 at 5:50 PM, Taher Alkhateeb <
> > >>> >> [hidden email]> wrote:
> > >>> >>
> > >>> >>> Hello Pradhan,
> > >>> >>>
> > >>> >>> Refactoring is exactly what we need and is a welcomed activity. I
> > >>> >>> think we should, however, try to avoid "big ideas" across the
> > entire
> > >>> >>> code base. The subject of your message is the reason why I say
> > that.
> > >>> >>>
> > >>> >>> So, if you want to start refactoring, I suggest to start with one
> > >>> >>> piece of code, study it careful, issue a JIRA, and provide a
> patch.
> > >>> >>> This should be focused similar to your notes on UtilCache.
> > >>> >>>
> > >>> >>> On Fri, Apr 13, 2018 at 12:14 PM, Pradhan Yash Sharma
> > >>> >>> <[hidden email]> wrote:
> > >>> >>> > Hello,
> > >>> >>> >
> > >>> >>> > While I was working on UtilCache.java file came across some
> > >>> >>> improvements,
> > >>> >>> > they are as follows:
> > >>> >>> >
> > >>> >>> > 1) Method and Variable access modifiers can be narrowed down to
> > >>> private
> > >>> >>> > access modifier.
> > >>> >>> >
> > >>> >>> > 2) Then AtomicLong can be given value 0L instead of 0.
> > >>> >>> >
> > >>> >>> > 3) Some Variables is used in both synchronized and
> unsynchronized
> > >>> >>> blocks,
> > >>> >>> > so they can be declared final. eg,
> > >>> >>> >
> > >>> >>> >
> > >>> >>> >
> > >>> >>> > *protected AtomicLong hitCount = new AtomicLong(0);
> > >>> >>> private
> > >>> >>> > final AtomicLong hitCount = new AtomicLong(0L);*
> > >>> >>> > One variable was able to get most of my attention is
> > >>> >>> >
> > >>> >>> > *                protected ConcurrentMap<Object, CacheLine<V>>
> > >>> >>> memoryTable
> > >>> >>> > = null;*
> > >>> >>> >
> > >>> >>> > This is used in synchronized and unsynchronized blocks, this
> > Object
> > >>> can
> > >>> >>> be
> > >>> >>> > converted into ThreadLocal or AtomicReference but it would
> > require
> > >>> >>> changes
> > >>> >>> > in the current implementation as well.
> > >>> >>> >
> > >>> >>> > Lastly, there is extensive use of for loops for iteration we
> can
> > use
> > >>> >>> Java 8
> > >>> >>> > Streams, Collector, and other functions to leverage implicit
> > looping
> > >>> >>> > mechanism.
> > >>> >>> >
> > >>> >>> >
> > >>> >>> > --
> > >>> >>> > Pradhan Yash Sharma
> > >>> >>>
> > >>> >>
> > >>> >>
> > >>>
> > >>
> > >>
> >
>

Trying to configure E-Mail in OFBiz

I had a look at this:

  partymgr/control/viewprofile

there is a button "send email", which does not seem to work.

In Contact.ftl (Line 96) there is a JavaScript  call
which seems to have missing quotes and maybe a problem
with formatting the E-Mail-address.




Best regards,

Ralf

[Jacques Le Roux]

Which version are you referring to?

BTW your email has been moderated, please subscribe to the MLs: http://ofbiz.apache.org/mailing-lists.html

Jacques


Le 02/05/2018 à 15:08, [hidden email] a écrit :

> Trying to configure E-Mail in OFBiz
>
> I had a look at this:
>
>    partymgr/control/viewprofile
>
> there is a button "send email", which does not seem to work.
>
> In Contact.ftl (Line 96) there is a JavaScript  call
> which seems to have missing quotes and maybe a problem
> with formatting the E-Mail-address.
>
>
>
>
> Best regards,
>
> Ralf
>

The current version: Apache OFBiz 16.11.04.

Ralf




Am 03.05.2018 um 08:48 schrieb Jacques Le Roux:

> Which version are you referring to?
>
> BTW your email has been moderated, please subscribe to the MLs:
> http://ofbiz.apache.org/mailing-lists.html
>
> Jacques
>
> 1
> Le 02/05/2018 à 15:08, [hidden email] a écrit :
>> Trying to configure E-Mail in OFBiz
>>
>> I had a look at this:
>>
>>    partymgr/control/viewprofile
>>
>> there is a button "send email", which does not seem to work.
>>
>> In Contact.ftl (Line 96) there is a JavaScript  call
>> which seems to have missing quotes and maybe a problem
>> with formatting the E-Mail-address.
>>
>>
>>
>>
>> Best regards,
>>
>> Ralf
>>
>
>

[Jacques Le Roux]

Ralf,

This was fixed w/ https://issues.apache.org/jira/browse/OFBIZ-7075 which was done before the R16 branch was released

Please double check,

I guess you are now subscribed to the dev ML? Please send only to the dev ML, no need to put me in copy ;)

Thanks

Jacques


Le 07/05/2018 à 23:11, [hidden email] a écrit :

> The current version: Apache OFBiz 16.11.04.
>
> Ralf
>
>
>
>
> Am 03.05.2018 um 08:48 schrieb Jacques Le Roux:
>> Which version are you referring to?
>>
>> BTW your email has been moderated, please subscribe to the MLs:
>> http://ofbiz.apache.org/mailing-lists.html
>>
>> Jacques
>>
>> 1
>> Le 02/05/2018 à 15:08, [hidden email] a écrit :
>>> Trying to configure E-Mail in OFBiz
>>>
>>> I had a look at this:
>>>
>>>     partymgr/control/viewprofile
>>>
>>> there is a button "send email", which does not seem to work.
>>>
>>> In Contact.ftl (Line 96) there is a JavaScript  call
>>> which seems to have missing quotes and maybe a problem
>>> with formatting the E-Mail-address.
>>>
>>>
>>>
>>>
>>> Best regards,
>>>
>>> Ralf
>>>
>>
>