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

Removed unnecessary constructor #1944

Closed
wants to merge 1 commit into from

Conversation

seujorgenochurras
Copy link

List extends Collection
this means that in that case they are literally the same

@seujorgenochurras
Copy link
Author

Ok, maybe it wasn't that unnecessary lol.
But why is it giving the error "Removed Constructor"?

@jhy
Copy link
Owner

jhy commented Apr 24, 2023

Because the super returns an ArrayList<E>, not an Elements, hence the override.

Please run the testcases prior to any further PRs.

@jhy jhy closed this Apr 24, 2023
@seujorgenochurras
Copy link
Author

seujorgenochurras commented Apr 25, 2023

Because the super returns an ArrayList<E>, not an Elements, hence the override.

Please run the testcases prior to any further PRs.

Maven tests run like butter.
I didn't understand, could you please clarify?

The super class is ArrayList, the constructor that I have removed utilizes the same super constructor already defined (the one that uses Collection instead)

@jhy
Copy link
Owner

jhy commented Apr 26, 2023

Maven tests run like butter.
I didn't understand, could you please clarify?

You should run mvn verify which will run all the verification plugins, including the japicmp plugin which checks backwards compatibility, and errors out here. In fact if you run that after making the change but without recompiling, you will see unit tests failing as well, with:

java.lang.NoSuchMethodError: 'void org.jsoup.select.Elements.<init>(java.util.List)'

The super class is ArrayList, the constructor that I have removed utilizes the same super constructor already defined (the one that uses Collection instead)

It's source compatible but not binary compatible, as compiled classes will be looking for the List signature method.

The reason both methods are there in the first place is because early on, they had different implementations. The List version kept a local ref to the list, while the Collection version made a copy. That was simplified later. (You can see the history for more details if you're interested.)

@seujorgenochurras
Copy link
Author

Wow, thank you for that.
I really wasn't expecting a response.
Thanks for the clarification

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.

None yet

2 participants