Things I check when I do a code review

Avatar

Avatar

Jörg_Hoh

Employee

Total Posts

3.0K

Likes

942

Correct Reply

1.0K

Avatar

Jörg_Hoh

Employee

Total Posts

3.0K

Likes

942

Correct Reply

1.0K
Jörg_Hoh
Employee

30-03-2017

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

Replies

Avatar

Avatar

smacdonald2008

Total Posts

12.7K

Likes

1.4K

Correct Reply

2.3K

Avatar

smacdonald2008

Total Posts

12.7K

Likes

1.4K

Correct Reply

2.3K
smacdonald2008

30-03-2017

Great community write up! 

Avatar

Avatar

kautuk_sahni

Community Manager

Total Posts

5.6K

Likes

1.0K

Correct Reply

1.1K

Avatar

kautuk_sahni

Community Manager

Total Posts

5.6K

Likes

1.0K

Correct Reply

1.1K
kautuk_sahni
Community Manager

30-03-2017

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

Avatar

Avatar

Sh1ju

Avatar

Sh1ju

Sh1ju

31-03-2017

Nice work and very useful

Avatar

Avatar

daniel_henriqu1

Avatar

daniel_henriqu1

daniel_henriqu1

31-03-2017

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

Avatar

Jörg_Hoh

Employee

Total Posts

3.0K

Likes

942

Correct Reply

1.0K

Avatar

Jörg_Hoh

Employee

Total Posts

3.0K

Likes

942

Correct Reply

1.0K
Jörg_Hoh
Employee

31-03-2017

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

Avatar

daniel_henriqu1

Avatar

daniel_henriqu1

daniel_henriqu1

01-04-2017

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

Avatar

Jörg_Hoh

Employee

Total Posts

3.0K

Likes

942

Correct Reply

1.0K

Avatar

Jörg_Hoh

Employee

Total Posts

3.0K

Likes

942

Correct Reply

1.0K
Jörg_Hoh
Employee

01-04-2017

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

Avatar

edubey

Avatar

edubey

edubey

02-04-2017

Nice and Important Stuff,

Thanks for sharing.

Avatar

Avatar

Ratna_Kumar

MVP

Avatar

Ratna_Kumar

MVP

Ratna_Kumar
MVP

03-04-2017

Thanks for sharing Jorg,

Its really helpful!!

~ Ratna.