Re: [GitHub] [roller] mbien opened a new pull request #96: Deprections, compiler warning fixes, local refactorings and JDK 17 support
On 15.07.21 10:12, Greg Huber wrote: You are correct, it is easy to read, and a good one liner. Alas, like most frameworks nowadays, we lean how the framework works (former) rather than programming (latter). having the main class WeblogEntry getTag() initially from the db in the correct order (by pubtime) would save some work. right. A few things could be put to "on write" instead of "on read", since data is read more often than written in typical web apps. Since JPA itself can return streams, we could go streams all the way to the DB without collections in between if a perf bottle neck is identified. Although, in this particular case of tags, sorting is basically a no-op perf wise since a blog entry has 4 tags or so - so i would not bother. I am running the blog (and 2 other JVMs) on a raspberry pi 3 with JFR and some performance metrics enabled and it is "fast enough for me". Rendering perf is ok (most time is spent in velocity) and once its in the cache, all this doesn't matter anymore. (I run a post-start task which warms up all entries in the feed to load everything into the cache) larger deployments like the apache blog would probably have proper caching in front of roller anyway without even hitting the roller cache. + the PR was large enough already. btw if you want to review a few commits of this PR please do so on github. -michael
Re: [GitHub] [roller] mbien opened a new pull request #96: Deprections, compiler warning fixes, local refactorings and JDK 17 support
yes when using streams you have to be careful to maintain readability,
but very often, they can be easier to read due to the fact that they are
more compact. Its fairly situational. I usually write one action per
line which makes it easier to read
public List getTags() {
return this.pojo.getTags().stream()
.sorted(new WeblogEntryTagComparator()) // by name
.map(WeblogEntryTagWrapper::wrap)
.collect(Collectors.toList());
}
now lets take a look at what roller had before:
public List getTags() {
// Sort by name
Set initialCollection = new TreeSet<>(new
WeblogEntryTagComparator());
initialCollection.addAll(this.pojo.getTags());
// iterate through and wrap
// we force the use of an ArrayList because it should be good
enough to cover
// for any Collection type we encounter.
ArrayList wrappedCollection = new
ArrayList(initialCollection.size());
Iterator it = initialCollection.iterator();
int i = 0;
while(it.hasNext()) {
wrappedCollection.add(i,WeblogEntryTagWrapper.wrap((WeblogEntryTag)
it.next()));
i++;
}
return wrappedCollection;
}
the main advantage is that streams are a stack of actions which are
evaluated later. So if you have many methods which would return
collections, you could consider making them return streams instead, so
that they become a slice on the stack of actions.
If the caller of the chain of methods would only have to get the first
element of the stream, only part of the chain would be evaluated - can be
much faster and less gc intensive compared to collections. There is also
the option of .parallel() which can make a difference in some situations.
lambdas might look foreign at first but you get used to them quickly. It
might help to think of them as if they would be anonymous inner classes.
- michael
On 15.07.21 07:32, Greg Huber wrote:
Probably guarantee streams will become the defacto way and their
overuse. Personally prefer the standard way, understanding loops etc
which would be common to other languages. The silly syntax is just
something else we have to remember.
I have seen some really complex one liners, with the arrow function,
and no explanation on what its doing. Clever? not.
Cheers Greg
On 11/07/2021 15:13, Michael Bien wrote:
no worries. I remember the first time i heard of java getting streams
my first thought was io too.
Streams are great as long you don't overuse them.
-michael
On 11.07.21 10:09, Greg Huber wrote:
Sorry you are right, was thinking more of io streams, rather than
java.util.Collection.stream. Says it closes it once read, and
should maintain the order.
too many ways of doing the same stuff.
On 11/07/2021 08:36, Michael Bien wrote:
Hi Greg,
those are java 8 functional streams* - not IO streams. So they
don't really have to be closed, the method collect() "runs" the
stream in this example by putting all results into a list. There
are no system resources involved which would have to be freed.
this code snipped creates a stream from the list, wraps each
element into a WeblogEntryWrapper, then creates a list again. In
future the pojo could directly return a stream instead of a list,
so the method would only add a step to the stream like a pipeline
(and return a stream too).
does this answer your question?
thanks for reviewing :)
best regards,
michael
*
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/package-summary.html
- - - -
https://mbien.dev
On 11.07.21 08:45, Greg Huber wrote:
Maybe more of a java question, how do these streams get closed as
wrappers are in used in the templates?
org.apache.roller.weblogger.pojos.wrapper;
public List retrieveWeblogEntries(boolean
publishedOnly) throws WebloggerException {
return
this.pojo.retrieveWeblogEntries(publishedOnly).stream()
.map(entry -> WeblogEntryWrapper.wrap(entry,
urlStrategy))
.collect(Collectors.toList());
}
I always use a try-with-resources on these.
On 10/07/2021 11:03, GitBox wrote:
mbien opened a new pull request #96:
URL: https://github.com/apache/roller/pull/96
Worked myself though the compiler warnings. This looks like a
lot of changes, but the individual changes are all very local.
* since most of the rawtype warnings were on Collections,
I updated a lot of the code to use more modern APIs instead of
only fixing the warnings (e.g List.of, streams, ...)
* extracted common reflection code to a utility class
* got rid of some deprecations
* some cleanup and improvements (~400 lines less code)
* JDK 17 support (runs still on 11)
Re: [GitHub] [roller] mbien opened a new pull request #96: Deprections, compiler warning fixes, local refactorings and JDK 17 support
Probably guarantee streams will become the defacto way and their
overuse. Personally prefer the standard way, understanding loops etc
which would be common to other languages. The silly syntax is just
something else we have to remember.
I have seen some really complex one liners, with the arrow function, and
no explanation on what its doing. Clever? not.
Cheers Greg
On 11/07/2021 15:13, Michael Bien wrote:
no worries. I remember the first time i heard of java getting streams
my first thought was io too.
Streams are great as long you don't overuse them.
-michael
On 11.07.21 10:09, Greg Huber wrote:
Sorry you are right, was thinking more of io streams, rather than
java.util.Collection.stream. Says it closes it once read, and should
maintain the order.
too many ways of doing the same stuff.
On 11/07/2021 08:36, Michael Bien wrote:
Hi Greg,
those are java 8 functional streams* - not IO streams. So they don't
really have to be closed, the method collect() "runs" the stream in
this example by putting all results into a list. There are no system
resources involved which would have to be freed.
this code snipped creates a stream from the list, wraps each element
into a WeblogEntryWrapper, then creates a list again. In future the
pojo could directly return a stream instead of a list, so the method
would only add a step to the stream like a pipeline (and return a
stream too).
does this answer your question?
thanks for reviewing :)
best regards,
michael
*
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/package-summary.html
- - - -
https://mbien.dev
On 11.07.21 08:45, Greg Huber wrote:
Maybe more of a java question, how do these streams get closed as
wrappers are in used in the templates?
org.apache.roller.weblogger.pojos.wrapper;
public List retrieveWeblogEntries(boolean
publishedOnly) throws WebloggerException {
return this.pojo.retrieveWeblogEntries(publishedOnly).stream()
.map(entry -> WeblogEntryWrapper.wrap(entry,
urlStrategy))
.collect(Collectors.toList());
}
I always use a try-with-resources on these.
On 10/07/2021 11:03, GitBox wrote:
mbien opened a new pull request #96:
URL: https://github.com/apache/roller/pull/96
Worked myself though the compiler warnings. This looks like a
lot of changes, but the individual changes are all very local.
* since most of the rawtype warnings were on Collections,
I updated a lot of the code to use more modern APIs instead of
only fixing the warnings (e.g List.of, streams, ...)
* extracted common reflection code to a utility class
* got rid of some deprecations
* some cleanup and improvements (~400 lines less code)
* JDK 17 support (runs still on 11)
Re: [GitHub] [roller] mbien opened a new pull request #96: Deprections, compiler warning fixes, local refactorings and JDK 17 support
no worries. I remember the first time i heard of java getting streams my
first thought was io too.
Streams are great as long you don't overuse them.
-michael
On 11.07.21 10:09, Greg Huber wrote:
Sorry you are right, was thinking more of io streams, rather than
java.util.Collection.stream. Says it closes it once read, and should
maintain the order.
too many ways of doing the same stuff.
On 11/07/2021 08:36, Michael Bien wrote:
Hi Greg,
those are java 8 functional streams* - not IO streams. So they don't
really have to be closed, the method collect() "runs" the stream in
this example by putting all results into a list. There are no system
resources involved which would have to be freed.
this code snipped creates a stream from the list, wraps each element
into a WeblogEntryWrapper, then creates a list again. In future the
pojo could directly return a stream instead of a list, so the method
would only add a step to the stream like a pipeline (and return a
stream too).
does this answer your question?
thanks for reviewing :)
best regards,
michael
*
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/package-summary.html
- - - -
https://mbien.dev
On 11.07.21 08:45, Greg Huber wrote:
Maybe more of a java question, how do these streams get closed as
wrappers are in used in the templates?
org.apache.roller.weblogger.pojos.wrapper;
public List retrieveWeblogEntries(boolean
publishedOnly) throws WebloggerException {
return this.pojo.retrieveWeblogEntries(publishedOnly).stream()
.map(entry -> WeblogEntryWrapper.wrap(entry,
urlStrategy))
.collect(Collectors.toList());
}
I always use a try-with-resources on these.
On 10/07/2021 11:03, GitBox wrote:
mbien opened a new pull request #96:
URL: https://github.com/apache/roller/pull/96
Worked myself though the compiler warnings. This looks like a
lot of changes, but the individual changes are all very local.
* since most of the rawtype warnings were on Collections, I
updated a lot of the code to use more modern APIs instead of only
fixing the warnings (e.g List.of, streams, ...)
* extracted common reflection code to a utility class
* got rid of some deprecations
* some cleanup and improvements (~400 lines less code)
* JDK 17 support (runs still on 11)
Re: [GitHub] [roller] mbien opened a new pull request #96: Deprections, compiler warning fixes, local refactorings and JDK 17 support
Hi Greg,
those are java 8 functional streams* - not IO streams. So they don't
really have to be closed, the method collect() "runs" the stream in this
example by putting all results into a list. There are no system
resources involved which would have to be freed.
this code snipped creates a stream from the list, wraps each element
into a WeblogEntryWrapper, then creates a list again. In future the pojo
could directly return a stream instead of a list, so the method would
only add a step to the stream like a pipeline (and return a stream too).
does this answer your question?
thanks for reviewing :)
best regards,
michael
*
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/package-summary.html
- - - -
https://mbien.dev
On 11.07.21 08:45, Greg Huber wrote:
Maybe more of a java question, how do these streams get closed as
wrappers are in used in the templates?
org.apache.roller.weblogger.pojos.wrapper;
public List retrieveWeblogEntries(boolean
publishedOnly) throws WebloggerException {
return this.pojo.retrieveWeblogEntries(publishedOnly).stream()
.map(entry -> WeblogEntryWrapper.wrap(entry,
urlStrategy))
.collect(Collectors.toList());
}
I always use a try-with-resources on these.
On 10/07/2021 11:03, GitBox wrote:
mbien opened a new pull request #96:
URL: https://github.com/apache/roller/pull/96
Worked myself though the compiler warnings. This looks like a lot
of changes, but the individual changes are all very local.
* since most of the rawtype warnings were on Collections, I
updated a lot of the code to use more modern APIs instead of only
fixing the warnings (e.g List.of, streams, ...)
* extracted common reflection code to a utility class
* got rid of some deprecations
* some cleanup and improvements (~400 lines less code)
* JDK 17 support (runs still on 11)
Re: [GitHub] [roller] mbien opened a new pull request #96: Deprections, compiler warning fixes, local refactorings and JDK 17 support
Maybe more of a java question, how do these streams get closed as
wrappers are in used in the templates?
org.apache.roller.weblogger.pojos.wrapper;
public List retrieveWeblogEntries(boolean
publishedOnly) throws WebloggerException {
return this.pojo.retrieveWeblogEntries(publishedOnly).stream()
.map(entry -> WeblogEntryWrapper.wrap(entry, urlStrategy))
.collect(Collectors.toList());
}
I always use a try-with-resources on these.
On 10/07/2021 11:03, GitBox wrote:
mbien opened a new pull request #96:
URL: https://github.com/apache/roller/pull/96
Worked myself though the compiler warnings. This looks like a lot of
changes, but the individual changes are all very local.
* since most of the rawtype warnings were on Collections, I updated a lot of the code to use more modern APIs instead of only fixing the warnings (e.g List.of, streams, ...)
* extracted common reflection code to a utility class
* got rid of some deprecations
* some cleanup and improvements (~400 lines less code)
* JDK 17 support (runs still on 11)
