Expand my Community achievements bar.

Things I check when I do a code review

Avatar

Employee Advisor

I collected a small list of topics I check when I do a code review. Knowing these items gives you the chance to avoid these pitfalls or to make better decisions :-)

https://cqdump.wordpress.com/2017/03/28/what-i-check-on-code-reviews/

Enjoy,
Jörg

10 Replies

Avatar

Administrator

Hi Jörg

This is really nice community work.

I have also pinned the post in the community so that it remains on top.

I will will go through the post and i would definitely share my comments on this.

 

PS: In future, whenever you would like to share good content with community just let me or Scott know  about it, we will pin the post.

~kautuk



Kautuk Sahni

Avatar

Level 4

Nice article, Jörg!

Regarding the item 3, how about using Lamba expressions to ensure that a ResourceResolver/Session will remain opened only within a limited scope?

Something similar to Groovy withReader/withWriter.

Regards,

Daniel.

Avatar

Employee Advisor

Hi Daniel,

I am not sure if the relevant classes already implement java.io.closable to work with the try-with statement of Java7.

Avatar

Level 4

To be honest, when I first wrote my answer I had a different solution in mind (I had forgotten about try-withResources).

What are your thoughts about

a "functional" interface representing the code in the limited scope

public interface Action<T> { public T perform(final ResourceResolver resourceResolver) throws PersistenceException; }

a OSGi utility service with a withResourceResolver method:

public <T> T withResourceResolver(final Action<T> action) throws PersistenceException, IllegalStateException { ResourceResolver resourceResolver = null; try { Map<String, Object> props = new HashMap<String, Object>(); props.put(ResourceResolverFactory.SUBSERVICE, SERVICE_NAME); resourceResolver = resourceResolverFactory.getServiceResourceResolver(props); return action.perform(resourceResolver); } catch (LoginException e) { logger.error("Error obtaining the Resource Resolver", e); throw new IllegalStateException(e); } finally { if (resourceResolver != null) { resourceResolver.close(); } } }

invoked by different classes:

String title = this.util.withResourceResolver((ResourceResolver resolver) -> { /* * Do here all the operations involving a resolver and return null or * any kind of object. */ /* For instance: */ Resource resource = resolver.getResource("/content/geometrixx-outdoors.html"); ValueMap props = resource.getValueMap(); return props.get(JcrConstants.JCR_TITLE, String.class); });

?

Regards,

Daniel.

Avatar

Employee Advisor

Hi,

I don't care what mechanism you use to make sure, that the session is closed after its usage. I am only interested in the fact, that it's eventually closed :-)

Jörg

Avatar

Level 10

Nice and Important Stuff,

Thanks for sharing.

Avatar

Level 10

Thanks for sharing Jorg,

Its really helpful!!

~ Ratna.