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

Possible minor optimization in case of package and class name ignores #68

Closed
vorburger opened this issue Feb 2, 2018 · 5 comments
Closed

Comments

@vorburger
Copy link
Contributor

while I was working on #67 for #64 I realized hat you could probably do a minor optimization... in case of package ignores (and the same goes for and class name ignores from my contribution; I've stuck to your style instead of mixing this idea into that change), then you should be able to return much earlier than the code currently does?

As is, the ignore is handled in the private checkToken() but that is per statement, right? So your checking entire packages or classes knowning that no violation in tem will ultimately ever be added to occurrences...

Move ignorePackages.contains(packageName) from checkToken to ignoreClass() (from my change), and the visit() method could return early if (ignoreClass()) ? Not sure what MethodVisitor it would have to turn, but I'm guessing there's a NOOP one.

@gaul
Copy link
Owner

gaul commented Oct 27, 2018

This may make sense. If you run modernizer on a large code base can you see if this optimization helps?

@vorburger
Copy link
Contributor Author

@gaul will do once I actually use modernizer-maven-plugin; still waiting for #69 ... ;-)

@gaul
Copy link
Owner

gaul commented Sep 1, 2019

@vorburger Can you investigate this?

@vorburger
Copy link
Contributor Author

@gaul Hallo! Unfortunately I meanwhile no longer work (on a project) with modernizer-maven-plugin, so probably won't get to this; sorry, but sure you understand. Therefore, from my side, absolutely no objection taken if you just close this issue now. If you think it could still be of interest / future for you / other users to do, keep it open - up to you.

@gaul
Copy link
Owner

gaul commented Oct 20, 2023

Closing this out as stale.

@gaul gaul closed this as completed Oct 20, 2023
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

2 participants