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

Fix JAMES-1815: Copy potentially immutable list to allow modifications #47

Merged
merged 2 commits into from Aug 17, 2016

Conversation

tfg13
Copy link
Contributor

@tfg13 tfg13 commented Aug 9, 2016

@@ -118,6 +118,9 @@ public String getDefaultDomain() throws DomainListException {
List<String> domains = getDomainListInternal();
Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to don't like variable re-affectation....

Here I would prefer a variable called "mutableDomains" to be created rather than seing domains reused.

By the way, nice spot.

@tfg13
Copy link
Contributor Author

tfg13 commented Aug 9, 2016

Ok, I changed the fix to introduce a new variable.
I also noticed something else: When domains is null, ImmutableList.copyof(domains) will crash with a NPE.
Also fixed that.

@chibenwa
Copy link
Contributor

chibenwa commented Aug 9, 2016

👍

1 similar comment
@rouazana
Copy link
Member

👍

@asfgit asfgit merged commit b0d973c into apache:master Aug 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants