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

tmf: remove unnecessary cast #176

Merged

Conversation

arfio
Copy link
Contributor

@arfio arfio commented Oct 24, 2024

This patch remove some casts that are showing as errors in eclipse.

I found two categories:

  • Cast from children classes to parent classes: I removed those ones since they are not really needed.
  • Cast after an instanceof check: It seems that in some recent version, the behaviour changed and these ones are sometimes classified as unnecessary. I found information on JEP 305 where that allows the following:
if (animal instanceof Cat cat)  {  
      out.println(cat.meow());  
}

However, in this case the following code indicates the cast as unnecessary:

IResource resource = getResource();
if (resource instanceof IFolder) {
    path = ((IFolder) resource).getFullPath();
}

IResource being a parent class of IFolder.
As I am not sure what really happens there, I have not made the change to remove those casts even though removing it seems to work.

@bhufmann
Copy link
Contributor

Does this work with Java 11? We still support older Eclipse targets (e.g. 2021-06, e4.20) that builds with Java 11.

@arfio
Copy link
Contributor Author

arfio commented Oct 24, 2024

I seem to have no errors building this with java 11 and eclipse 2021-06, e4.20. However I have a couple of errors with this PR: #174 for the changes added to the CompositeHostModel.

Signed-off-by: Arnaud Fiorini <[email protected]>
@arfio
Copy link
Contributor Author

arfio commented Oct 24, 2024

After verifying the other cases, the compiler was right in identifying most of these as unnecessary because in every instance, the method called after casting was present in the parent class.

Copy link
Contributor

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks.

Copy link
Contributor

@MatthewKhouzam MatthewKhouzam left a comment

Choose a reason for hiding this comment

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

Less code is a yes

@MatthewKhouzam MatthewKhouzam merged commit 871c450 into eclipse-tracecompass:master Nov 4, 2024
4 checks passed
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