Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/8 #9

Merged
merged 2 commits into from
Mar 3, 2014
Merged

Feature/8 #9

merged 2 commits into from
Mar 3, 2014

Conversation

joaollq
Copy link
Contributor

@joaollq joaollq commented Nov 26, 2013

Improves #8.

Still can have issues if with the automatic number generator if the operator set the counters below the maximum number attributed at a given moment.

Fixing that problem would require a dangerous operation of looking at every entry present and check if the numbers already exist. However this implies a great change in the code which can lead to non desirable behaviours

final String reference = correspondenceEntryBean.getReference();
final CorrespondenceEntry entry = correspondenceEntryBean.getEntry();

if (CollectionUtils.select(correspondenceEntryBean.getMailTracking().getActiveEntries(correspondenceType),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common's CollectionUtils, while perfectly functional, should be avoided.

You could instead use Guava's Iterables class see here to perform this operation. It provides a much cleaner API, and lazily evaluates the collection, in a much more efficient manner.

Note that I am not saying you should change what you have done here, just leaving it for future reference :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just saw it CollectionUtils was being used in some parts of the code, so I just followed it.

But I'll use Iterables next time, thanks for the tip ;)

@Luis-Cruz Luis-Cruz merged commit 8d976f3 into ist-dsi:develop Mar 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants