spectrum icon indicating copy to clipboard operation
spectrum copied to clipboard

Experimental feature for unboxing.

Open ashleyfrieze opened this issue 8 years ago • 6 comments
trafficstars

This is what I had in mind for #103.

@greghaskins - does it majorly suck, or does it add value?

ashleyfrieze avatar Sep 04 '17 08:09 ashleyfrieze

Codecov Report

Merging #122 into master will decrease coverage by 0.15%. The diff coverage is 100%.

@@             Coverage Diff             @@
##             master    #122      +/-   ##
===========================================
- Coverage     99.45%   99.3%   -0.16%     
+ Complexity      349     335      -14     
===========================================
  Files            43      41       -2     
  Lines           739     718      -21     
  Branches         22      22              
===========================================
- Hits            735     713      -22     
- Partials          4       5       +1
Impacted Files Coverage Δ Complexity Δ
...rc/main/java/com/greghaskins/spectrum/Unboxer.java 100% <100%> (ø) 2 <2> (?)
.../internal/configuration/TaggingFilterCriteria.java 96.55% <0%> (-3.45%) 17% <0%> (ø)
...com/greghaskins/spectrum/internal/hooks/Hooks.java 97.29% <0%> (-0.21%) 20% <0%> (-2%)
.../main/java/com/greghaskins/spectrum/Configure.java 100% <0%> (ø) 11% <0%> (-1%) :arrow_down:
.../java/com/greghaskins/spectrum/internal/Child.java 100% <0%> (ø) 3% <0%> (+1%) :arrow_up:
...n/java/com/greghaskins/spectrum/internal/Spec.java 100% <0%> (ø) 11% <0%> (-1%) :arrow_down:
...ectrum/internal/configuration/ConfiguredBlock.java 100% <0%> (ø) 6% <0%> (ø) :arrow_down:
.../com/greghaskins/spectrum/internal/hooks/Hook.java 100% <0%> (ø) 1% <0%> (-1%) :arrow_down:
.../java/com/greghaskins/spectrum/internal/Suite.java 100% <0%> (ø) 53% <0%> (-2%) :arrow_down:
...kins/spectrum/internal/hooks/NonReportingHook.java
... and 3 more

codecov[bot] avatar Sep 04 '17 08:09 codecov[bot]

Nice.

Does it work with concrete types or only interfaces? I'm thinking people would really like to use this for mocks, which are frequently (with Mockito at least) concrete types.

Still mulling over the unbox() entry point. Not the worst thing, but another import/function for people to learn. My first thought was overloading let like let(Something.class, () -> new Something()).

There may also be some sneaky tricks to even avoid passing in that Class object; I feel like maybe Hamcrest or something has some code that just works with generics somehow – unbox(let(() -> new Something()); would be pretty slick.

greghaskins avatar Sep 07 '17 18:09 greghaskins

The inbox function as provided can be used to surround any supplier, of which both let and junitMixin are such... but we could provide overloads to wrap it further.

It can ONLY work for interfaces. This is a technical constraint to do with dynamic proxies, but also makes sense if you think about how concrete types can have accessible instance variables and multiple constructors, which even Mockito has limitations over.

The type must be specified in the call because the stuff you mentioned about inferring type only works for generic types themselves, not for actual Class objects. In other words, to know at run time the interface to supply, you need a Class object. Even Mockito.mock requires a class object as input.

ashleyfrieze avatar Sep 07 '17 20:09 ashleyfrieze

My first thought was overloading let like let(Something.class, () -> new Something()).

I've extended my POC to have the equivalent of this - I reverse the order of parameters from the above, because that's what felt natural, but it's easily refactored. I did it for junitMixin and let, but left the unboxer documented and public as this tool could conceivably be used by end-users for their own combinations of things.

How does it look?

ashleyfrieze avatar Sep 08 '17 08:09 ashleyfrieze

Understood ✅ . A couple thoughts:

  • If it only works with interfaces, we should fail fast (throw an IllegalArgumentException) to help people realize that quickly, rather than getting a stacktrace from deep in the standard library.
  • Since this is experimental, and limited to just interfaces (for now – back of my mind is already wondering how to use ByteBuddy down the road...), let's just use a single entry point like unbox. If this feature takes off and people really like it, we can add the convenience overloads of let and junitMixin.
  • I don't know what the best name for unbox() would be, but I can't think of anything significantly better. unwrap(), proxy(), get(), and from() come to mind; I'm not in love with any of them.

greghaskins avatar Sep 09 '17 23:09 greghaskins

So, I should remove the overloads of let and junitmixin? It is easy to check if the passed in class is an interface.

Naming the method was hard. It's an auto get or an unwrapper or a de supplier or... nothing sounded good. Accessor?

ashleyfrieze avatar Sep 10 '17 00:09 ashleyfrieze