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

New reflection API Still lacks several Features of old reflection API #130

Open
3 of 16 tasks
Rdeisenroth opened this issue Dec 9, 2023 · 0 comments
Open
3 of 16 tasks
Labels
help wanted Extra attention is needed invalid This doesn't seem right priority: high An issue with high priority

Comments

@Rdeisenroth
Copy link
Member

Rdeisenroth commented Dec 9, 2023

Why we shouldn't ditch the old Reflection API just yet

The Problem

The new Reflection API was build from the Ground using better Patterns and being much more versatile in it's application. At least in theory. In #74 we deprecated the reflection API stating that the new API is ready to replace it. However after using the new reflect API for a more complex Assignment where Students have to declare their own Classes, i realize it's still very much incomplete.

new vs old API

Here is a comprehensive List of things that are currently missing in the new API but are present in the old API. Until all of these are ported to the new API, we should imo not remove or deprecate the old one.

  • documentation. Like seriously, half of the code doesn't even have Javadoc...
  • checking if a method from an interface is implemented
  • verifying no blacklisted statements were used
  • automatically invoking methods with random values to see if they are callable at all
  • still being able to invoke methods even if students miss or reorder parameters (omitting the missing args)
  • automatically writing tests for getters/setters
  • automatically verifying the method signature, not just the parameters (including throws, generic type, maybe annotations)
  • automatically test that constructor saves parameters in private attributes
  • automatically generate instances of interfaces and abstract classes using Mockito
  • bridge for Mockito verifiers (e.g. somethink like methodLink.verify(mock,times(5))
  • bridge for Mockitos when (e.g. methodLink.when(mock,<Answer>))
  • lazy evaluation (Class tested had .resolve(), currently needs hacky workaround using sth like Suppliers.memoize() from Guava)
  • utility methods for reflective access such as typeLink.setField(obj) and typeLink.setFieldTyped(obj,clazz)
  • unified error messages. Almost every single assertion command requires a pre comment supplier without providing any useful comment itself.
  • easy to understand import structure: The seperation of Assertions2, Assertions3, Assertions4, etc. Makes it very hard to understand where the desired Utility Method lies. We should have all assertion methods at least in one appropriately named file.
  • syntactic sugar. There are many instances where the old API is just more convenient to use. Especially the fact that you have to use a Matcher for almost everything without an overload to provide the actual value instead. For example in order to create an instance of a class you used to be able to just call new ClassTester(clazz).resolveInstance(param1,...) which first tried to create an instance by using the constructor, then as a fallback returned a mock. With the new API you have to
    • get a TypeLink tl By calling BasicTypeLink.of(clazz);
    • get a ConstructorLink cl by calling Assertions3.assertConstructorExists(tl,BasicReflectionMatchers.sameTypes(args)); (for args you then have to create a Type link for every arg again...)
    • manually handle the errors and create something like this:
/**
 * Creates an instance of the given class using the given constructor and the given arguments. If this fails, a mock
 * instance is created instead with "CALLS_REAL_METHODS" enabled.
 *
 * @param classLink       the class to instantiate
 * @param constructorLink the constructor to use
 * @param args            the arguments to try to pass to the constructor
 * @return the created instance
 */
@SafeVarargs
public static Object createInstance(
    BasicTypeLink classLink,
    ObjectCallable<BasicConstructorLink> constructorLink,
    ObjectCallable<Object>... args
) {
    Object instance;
    try {
        instance = constructorLink.call().invoke(Arrays.stream(args).map(Assertions2::callObject).toArray());
    } catch (Throwable e) {
        instance = mock(classLink.reflection(), CALLS_REAL_METHODS);
    }
    return instance;
}

What now?

I think i will still use parts of the old API for my personal Tests until the new one is matured. However i really prefer the new structure as it has a lot more potential. So porting the old functions over should be one of our top priorities this year as i started in #129.

@Rdeisenroth Rdeisenroth added help wanted Extra attention is needed invalid This doesn't seem right priority: high An issue with high priority labels Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed invalid This doesn't seem right priority: high An issue with high priority
Projects
None yet
Development

No branches or pull requests

1 participant