ponder-lab / Optimize-Java-8-Streams-Refactoring

Refactorings for optimizing Java 8 stream client code for greater parallelism and efficiency.
http://cuny.is/streams
Eclipse Public License 1.0
8 stars 7 forks source link

Arbitrary entry points #114

Closed yiming-tang-cs closed 6 years ago

yiming-tang-cs commented 6 years ago

accept arbitrary entry point #98.

khatchad commented 6 years ago

@saledouble Can you please place a reference to the issue number in the pull request description?

yiming-tang-cs commented 6 years ago

You are right. Please give me some time to fix them.

khatchad commented 6 years ago

You should deal with the checked exception, and not be converting it to a runtime exception. It's probably checked for a reason, and there may be a way of handling the error. Most likely this method should throw this exception, and somewhere up the call path, we should log a warning, use whatever entry points we've found thus far, and move on.

On Wed, 2017-11-22 at 17:42 +0000, Grace Tang wrote:

@saledouble commented on this pull request.

I pushed something new. :) I hope it can pass in travis.


In edu.cuny.hunter.streamrefactoring.core/src/edu/cuny/hunter/streamrefactoring/core/utils/Util.javahttps://github.com/ponder-lab/Java-8-Stream-Refactoring/pull/114#discussion_r152617799:

  • if (!AnalysisUtils.isJDKClass(klass)) {
  • // iterate over all declared methods
  • for (com.ibm.wala.classLoader.IMethod method : klass.getDeclaredMethods()) {
  • try {
  • if (!(method instanceof ShrikeCTMethod)) {
  • throw new RuntimeException("@EntryPoint only works for byte code.");
  • }
  • // if method has an annotation
  • for (Annotation annotation : ((ShrikeCTMethod) method).getAnnotations(true)) {
  • if (isEntryPointClass(annotation.getType().getName())) {
  • result.add(new DefaultEntrypoint(method, classHierarchy));
  • break;
  • }
  • }
  • } catch (Exception e) {

Because ((ShrikeCTMethod) method).getAnnotations(true)) may throw InvalidClassFileException. Should I change this line?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHubhttps://github.com/ponder-lab/Java-8-Stream-Refactoring/pull/114#pullrequestreview-78507396, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AB9DP5h6CajtECx32ZYESavYgsA0HakMks5s5F0IgaJpZM4Qm3xG.

yiming-tang-cs commented 6 years ago

Did you mean log a warning in the catch statement:

// iterate over all declared methods
for (com.ibm.wala.classLoader.IMethod method : klass.getDeclaredMethods()) {
    try {
        if (!(method instanceof ShrikeCTMethod)) {
            throw new IllegalArgumentException("@EntryPoint only works for byte code.");
        }
        // if method has an annotation
        for (Annotation annotation : ((ShrikeCTMethod) method).getAnnotations(true)) {
            if (isEntryPointClass(annotation.getType().getName())) {
                result.add(new DefaultEntrypoint(method, classHierarchy));
                break;
        }
        }
    } catch (InvalidClassFileException e) {
        LOGGER.log(Level.WARNING, "The class is invalid", e);
    } catch (IllegalArgumentException e) {
        LOGGER.log(Level.WARNING, "@EntryPoint only works for byte code.", e);
}
khatchad commented 6 years ago

No, that method should not handle the exception. Please review exception handling in the Horstmann text. That method may be used from other places that need to handle the exception in a different way.

yiming-tang-cs commented 6 years ago

So you want the method findEntryPoints(IClassHierarchy classHierarchy) throws exception and let other methods, which call findEntryPoints(), handle the exception?

khatchad commented 6 years ago

Right. That method is too low-level to decide what to do with it.