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

Add MethodInvocationCounter Metric to Track Method Invocations within Classes #120

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

BrenoFariasdaSilva
Copy link
Contributor

Overview

This PR introduces the MethodInvocationCounter metric, a new ClassLevelMetric designed to analyze and report on method invocation patterns within Java classes. The metric captures the frequency of invocations for each method within a class, providing insights into method dependencies and utilization.

Key Changes

  1. CKClassResult.java (REFACTOR):

    • New Data Member: Added a private string field methodInvocations to store the formatted string representing method invocation statistics for each class.
    • Accessor and Mutator Methods: Added setMethodInvocation(String methodInvocations) and getMethodInvocations() to handle this new data. This allows the stored invocation data to be set during metric calculation and later retrieved for output.
  2. MethodCounter.java (FEAT):

    • New Utility Class: Introduced to assist with counting and formatting method invocations. This class includes nested classes and methods that:
      • Store method invocation data (MethodInformation class).
      • Sort and format the collected data into a readable format (formatResult and toFormattedString methods).
    • This utility class provides a centralized way to manage method invocation data, ensuring consistency and reusability across different parts of the metric implementation.
  3. MethodInvocationCounter.java (FEAT, FIX, REFACTOR):

    • Metric Implementation: This file defines the MethodInvocationCounter class which implements CKASTVisitor and ClassLevelMetric. It contains the logic to visit method declarations and invocations, and it populates the MethodCounter.MethodInformation data structure with this information.
    • Visitor Methods: Implements the visit(MethodDeclaration node) to set the current method context and visit(MethodInvocation node) to record invocations within the context of the current method.
    • Set Results: Overrides setResult(CKClassResult result) to format and set the method invocation data on the CKClassResult.
  4. ResultWriter.java (REFACTOR):

    • Output Addition: Modified to include the methodInvocations in the output of the CK tool. This ensures that the collected method invocation data is written out alongside other metrics.

Implementation Details

The MethodInvocationCounter leverages the AST (Abstract Syntax Tree) to visit and analyze MethodDeclaration and MethodInvocation nodes within Java source files. For each class, it maintains a mapping of methods and the methods they invoke, along with the invocation counts. This data is then formatted into a human-readable string that summarizes the invocation patterns, making it easy to identify which methods are central to the functionality of the class.

Output

Here is an example of the Output generated when using CK to generate the class.csv for the ZooKeeper repository:
image

Usefulness

The MethodInvocationCounter metric is particularly useful for:

  • Identifying Key Methods: By highlighting methods with high invocation counts, developers can quickly identify the core components of a class.
  • Refactoring Opportunities: Methods that are heavily dependent on others or have high complexity might be candidates for refactoring to improve modularity and maintainability.
  • Optimizing Performance: Understanding method invocation dynamics can help in optimizing performance-critical parts of the application.
  • Documentation and Understanding: This metric provides a quick overview of class behavior, which can be beneficial for new developers or in documenting the system.

Conclusion

The addition of the MethodInvocationCounter provides valuable insights into method usage patterns, making it a useful tool for developers looking to maintain and improve the quality of their Java applications.

Copy link
Owner

@mauricioaniche mauricioaniche left a comment

Choose a reason for hiding this comment

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

This MR is great, thanks for the feature!

However, this one makes the output way larger. Therefore, this feature should be off by default, and enabled only optionally!

@BrenoFariasdaSilva
Copy link
Contributor Author

BrenoFariasdaSilva commented Apr 24, 2024

Hi again, @mauricioaniche.
So, the changes were made. Basically, the CK and Runner classes were modified to handle a new boolean execution argument (verbose).
In addition, the ClassLevelMetric interface was modified to include a new method named isVerbose, whose default return is false. With that in mind, any new class metric can overload this method and set the return value to true, which was done in the MethodInvocationCounter class.
If the verbose execution argument is set to true, every metric will be processed. Otherwise, it checks if the metric is not verbose, so it can be processed.

I acknowledge that this PR is massive. If you prefer, i can split it into two PRs: One for the verbose execution argument and another for the new class metric itself (MethodInvocationCounter).
Also, keep in mind that the build is failing. That's due to the fact that my older PR that fixes the build was not approved yet (#115). It's a simple PR, so it's fast to analyze.

@BrenoFariasdaSilva
Copy link
Contributor Author

BrenoFariasdaSilva commented Jun 7, 2024

Hi @mauricioaniche. The readme's conflict is solved :).

Copy link
Contributor Author

@BrenoFariasdaSilva BrenoFariasdaSilva left a comment

Choose a reason for hiding this comment

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

All unnecessary changes were undone

Copy link
Contributor Author

@BrenoFariasdaSilva BrenoFariasdaSilva left a comment

Choose a reason for hiding this comment

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

The unnecessary changes were undone.

Copy link
Owner

@mauricioaniche mauricioaniche left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

My main concern is about the new metric being a very long string. I think we should:

  • Make this feature disabled by default, so we don't produce such large extra data for people that are already using CK
  • Output it to a different file, so the CSV file stays short and sweet

@@ -208,10 +209,10 @@ in the directory and use them to better resolve types. `Max files per partition`
of the batch to process. Let us decide that for you and start with 0; if problems happen (i.e.,
out of memory) you think of tuning it. `Variables and field metrics` indicates to CK whether
you want metrics at variable- and field-levels too. They are highly fine-grained and produce a lot of output;
you should skip it if you only need metrics at class or method level. Finally, `output dir` refer to the
you should skip it if you only need metrics at class or method level. Aditionally, `output dir` refer to the
Copy link
Owner

Choose a reason for hiding this comment

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

There's a typo here, Additionally

directory where CK will export the csv file with metrics from the analyzed project.
Optionally, you can specify any number ignored directories, separated by spaces (for example, `build/`).
By default, `.git` and all other hidden folders are ignored.
By default, `.git` and all other hidden folders are ignored. Finally, the `verbose flag` tells CK if it must process metrics tagged as large outputs. If you are not interested in the detailed output of the metrics, you can set it to false.
Copy link
Owner

Choose a reason for hiding this comment

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

verbose flag, flag shouldn't be highlighted

import java.util.*;
import java.util.stream.Collectors;

public class MethodCounter {
Copy link
Owner

Choose a reason for hiding this comment

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

The string produced by this class can be really long. I'm not sure I'd like to mix the CSV, which is mostly numbers with this super large string. Also, given that this string can be quite long, just concatenating it to the CSV might make the CSV invalid. We'd need to use proper escaping here. An alternative would be to output this to a separate file. And make this feature disabled by default, so we don't start producing large data files to people that have been using CK so far (and do not expect it).

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.

2 participants