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

XPathContext should be immutable #19

Closed
carlosmiranda opened this issue Mar 21, 2014 · 9 comments
Closed

XPathContext should be immutable #19

carlosmiranda opened this issue Mar 21, 2014 · 9 comments

Comments

@carlosmiranda
Copy link
Contributor

The Javadoc for XPathContext helpfully tells us the following: "The class is immutable and thread-safe." This is a blatant lie. :)

  1. It's not annotated with @Immutable. And even if it were, it wouldn't work because...
  2. The fields map and contexts are instantiated with mutable types.
  3. The merge method creates a new instance and manipulates its map field before returning it.

We need to fix that, or change the Javadoc to suit. If we do need to fix it, you may assign it to me - I have a fix ready. :)

@yegor256
Copy link
Member

totally agree, we should fix it and annotate with @Immutable, thanks for reporting

@yegor256
Copy link
Member

@dmarkov please pay for this report

@dmarkov
Copy link

dmarkov commented Mar 21, 2014

we'll find someone to do this task, soon

carlosmiranda added a commit to carlosmiranda/jcabi-xml that referenced this issue Mar 22, 2014
@carlosmiranda
Copy link
Contributor Author

I created a pull request for it at #20 .

ghost pushed a commit that referenced this issue Mar 23, 2014
#20: pull request Issue #19 Made XPathContext immutable
@carlosmiranda
Copy link
Contributor Author

@dmarkov , @yegor256 this report hasn't been paid for yet...

@dmarkov
Copy link

dmarkov commented Mar 24, 2014

@dmarkov please pay for this report

@yegor256 of course. @carlosmiranda many thanks for reporting, I just topped your account up for 15 mins, payment ID: 37432905

@dmarkov
Copy link

dmarkov commented Mar 24, 2014

@carlosmiranda this is your task now, please help. The budget of it is 30 mins

@carlosmiranda
Copy link
Contributor Author

We're done here, merged pull request at #20.

@dmarkov
Copy link

dmarkov commented Mar 24, 2014

@carlosmiranda Thanks for your contribution, 30 mins was added to your account, payment ID is 37433380

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants