Skip to content

Conversation

@AlpTorac
Copy link
Contributor

@AlpTorac AlpTorac commented Sep 3, 2024

Summary

Complements the preceding pull request #9 (till acc918c), which mainly contains refactoring changes for similarity checking mechanisms, with several changes tackling various issues:

  • Adding null checks that prevent several NullPointerExceptions (NPEs)
    • Includes some issues within tests for Teammates case study
  • Fixing the symmetry of similarity checking (i.e. if obj1 is similar to obj2, then obj2 must be similar to obj1), such as 30f25bd
  • Implementation of some missing case methods in similarity switches (i.e. methods that encapsulate the definition of similarity of Java model elements)
  • Unification of certain conditionals (if-blocks etc.) via extracted utility classes
    • Improves consistency across different case methods
  • Splitting float and double literal comparison methods, in order to make sure that their comparison is correct, especially regarding their NaN value (ef7e1f4)
  • Fixed the issue regarding the LocalVariableStatement "reader" in the case study tests for TeaStore (d7e7d98)
  • Removing some redundant methods/constructors

The NPEs handled here can occur when certain attributes of certain Java-Model elements (in form of EObject instances) are not set properly. To circumvent such NPEs, null checks were added to Java-related "inner switches". Concretely, at positions where certain getter/access methods could return null. These null checks respect the symmetry of the similarity checking and some of them will "fail early", if a difference is found such that the given objects cannot be similar.

Since some files were also formatted in the process, ignoring space and commentary differences makes the important changes more visible.

Known Issues

  • Some Java model element interfaces are still not fully covered by case methods:
    • Although this is not an issue for any currently implemented Java model elements, it may cause problems if newer Java model elements are integrated
  • Validity of Java models are not fully checked before similarity checking, which can cause exceptions within the similarity checking process:
    • Java model elements, which are capable of referencing themselves (such as implementors of Reference interface), can cause StackOverflowException; as similarity checking cannot detect all cycles

from SimilarityChecker and related classes
Implement requests and handlers for certain similarity checking operations used in Java similarity switches

Helps centralise the said operations

Helps encapsulates some parameters that would otherwise unnecessarily clutter similarity checker and similarity switches
Builds upon the ...similarity.base package, extends and complements its elements with EMFtext

Provides exemplary similarity checking requests and handlers (using them is not mandatory)
For Java-related similarity switches

Currently empty

Can help separate Java-related similarity switches from the rest in the future
to keep the logging logic for Java-related inner switches in one place
For Java-related inner switches to not implement some mutual operations multiple times
Created for Java-related similarity checking
For PCM-related similarity switches
For PCM-related inner similarity switches
For PCM-related similarity checker

Use the extracted structure
For ID-based similarity checking of PCMs

A separate similarity checker implementation will no longer be necessary

To be integrated in the following future commits
For Java-related similarity checking
For logging Java-related inner switches
To their own files without modifying them

Non-functional commit
Rename SimilaritySwitch to JavaSimilaritySwitch to indicate its intent better

Use the extracted inner switches in the JavaSimilaritySwitch

Non-functional commit
To create new Java-related similarity switches

Non-functional commit
For Java-related inner switches that create further switches

Non-functional commit
Use extracted structure in the Java-related inner switches

Use implemented interfaces in the Java-related inner switches

Non-functional commit
For Java-related similarity checking

Non-functional commit
Rename SimilarityChecker to JavaSimilarityChecker

Use the extracted structure in the JavaSimilarityChecker

Delegate similarity switch creation by using a new similarity switch request

Non-functional commit
Adapt classes affected by the changes to the Java-related part of the similarity checking

Non-functional commit
To their own files without modifying them

Non-functional commit
Use the extracted structure in PCM-related similarity switch

Use extracted inner switches in PCM-related similarity switch

Non-similarity switch
@AlpTorac AlpTorac force-pushed the fi-ref-splitted-null-check branch from a56ab37 to 42c0418 Compare February 10, 2025 14:17
Extract utility classes for similarity checking

Done to help maintain consistency across similarity switches, since there are many comparisons that should be performed the same way.

This also spares having to implement long/compicated if-blocks, such as null checks that care about either both elements being null or none.
Use the utility classes instead
Use ILoggableSwitch from ILoggableJavaSwitch

Adapt affected classes
There are certain Commentable sub-types used by JaMoPP, which may use a Block instance, in order to store their Statement instances. If this is the case for both LocalVariableStatement (LVS) instances that are being compared, the said Block instances will be their direct container, rather than the intended one.

For example: If a LVS is to be contained within a TryBlock, it is instead contained within a Block instance first, which is then contained in the TryBlock instance (TryBlock -> Block -> LVS; not TryBlock -> LVS).

The changes made to the comparison of said containers makes sure that LVS comparison accounts for this effect. It is supposed to bypass the containers in-between, which are not the intended to be compared.
Retrieve their direct containers and let similarity checking decide whether they are similar.

Both case methods can be asymmetric in some cases, since the container of the first element is checked before the container of the compare element (the second element).
Review required

There was no case method for comparing AdditionalField instances, which are not fully initialised. This caused similarity checking to potentially return null.
Since floats and doubles can be different in some regards, their compare methods are separated. The wrapper class Float is used in compareFloat instead of Double.
Review required

There was no case method for comparing PackageImport instances, which are not fully initialised. This caused similarity checking to potentially return null.
Review required

There was no case method for comparing StaticClassifierImport instances, which are not fully initialised. This caused similarity checking to potentially return null.
Let normalization and string comparing logic deal with potentially null strings, since a string being null and another one being empty ("") can change the outcome of the comparison.
Review required

There was no case method for comparing EmptyModel instances. This caused similarity checking to potentially return null.
@AlpTorac AlpTorac changed the title FirstInstance post refactoring null checks FirstInstance post refactoring changes Feb 16, 2025
@AlpTorac AlpTorac marked this pull request as ready for review February 16, 2025 16:53
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.

1 participant