Ref types should be interfaces (BucketRef -> IBucket)

[rix0rrr]

I'm running into issues right now modeling ELBv2, and I think modeling Refs as interaces is more natural. Hear me out.

ELBv2

ELBv2 comes in two flavors: Application Load Balancer and Network Load Balancer. They're juuust different enough in terms of parameters and things you can configure on them that I want to separate them out, so I can have different properties and methods for each type.

(Examples for the doubtful: "enable access logging" only exists on an ALB, not an NLB. Only ALBs have Security Groups and so are Connectable, NLBs are not. ALBs have a protocol selector, NLBs always just have "TCP", which has bearing on optionality of parameters. Etc.)

However, since we also need to have the Ref types, this leaves me with the following inheritance diagram:

                       ┌────────────────────────┐                      
                       │   ??? Construct ???    │                      
                       └────────────────────────┘                      
                                    ▲                                  
               ┌────────────────────┴──────────────────┐               
               │                                       │               
               │                                       │               
┌─────────────────────────────┐         ┌─────────────────────────────┐
│ ApplicationLoadBalancerRef  │         │   NetworkLoadBalancerRef    │
└─────────────────────────────┘         └─────────────────────────────┘
               △                                       △               
               │                                       │               
┌─────────────────────────────┐         ┌─────────────────────────────┐
│   ApplicationLoadBalancer   │         │     NetworkLoadBalancer     │
└─────────────────────────────┘         └─────────────────────────────┘

It's hard for me to share implementation between the two sides of this divide, and I end up copy/pasting a bunch of code. Yes, I'm aware I can do an inner object. I think it just adds more code (basically duplicate all lines yet again, and add forwarding functions on the outer class), and is not worth it in this case.

On the other hand, if I had interfaces, I can do the following:

┌─────────────────────────────┐              ┌─────────────────────────────┐
│  IApplicationLoadBalancer   │              │    INetworkLoadBalancer     │
└─────────────────────────────┘              └─────────────────────────────┘
               △                                            △               
               │                                            │               
               │                                            │               
               │       ┌────────────────────────┐           │               
               │       │    BaseLoadBalancer    │           │               
               │       └────────────────────────┘           │               
               │                    △                       │               
               │                    │                       │               
               │     ┌──────────────┴─────────────────┐     │               
               │     │                                │     │               
               │     │                                │     │               
               │     │                                │     │               
      ┌────────┴────────────────────┐  ┌────────────────────┴────────┐      
      │   ApplicationLoadBalancer   │  │     NetworkLoadBalancer     │      
      └─────────────────────────────┘  └─────────────────────────────┘      

And similar for ImportedApplicationLoadBalancer. Much more natural, and I get to reuse some things that are common between load balancers, such as Attributes.

VPCs

I'm also thinking a bit about VPCs, and the myriad of VPC setups that @moofish32 is encountering as he's dealing with power user setups. We can possibly allow all of them in our current implementation; it would far overcomplicate the design for the 90% case of users, so we should allow a mechanism where people can write new kinds of VPC constructs.

Yes, right now we have VpcNetworkRef which they could extend, but I would argue that this class already ties way too much into implementation details that might break or complicate actual other VPC implementors.

For example, it dictates presence of member variables that are only used in implementations of a subnet selector and export mechanism defined in that class, both of which a different implementor might want to implement differently. To properly allow for a broad set of implementations, only the minimal set functions should be exposed and they should all be abstract--in which case the class is nothing more than an interface anyway, except with the restriction that you can't inherit from anything while implementing it.

Consequences

The import() and export() methods should go into the concrete type, not the Ref type. But I'd argue this makes more sense anyway. Exporting/importing is a feature of the classes we're providing, and classes 3rd parties write might or might not provide the same capabilities. And even export/import makes more sense on the concrete class with an interface than with this artificial Ref type. Examples:

// Need to get a Bucket-like thing (typed as BucketRef)
new Bucket()  
BucketRef.import()

// VS

// Need to get a Bucket-like thing (typed as IBucket)
new Bucket()
Bucket.import()
// Same class both times! You get a Bucket-like thing from Bucket!

Wrap-up

It seems so much more correct to me to be using interfaces instead of base classes anyway that I can't even remember why we weren't doing that in the first place. Was it the extra I at the start which will confuse/irk people in Javaland? Because I'm not sure that's a good reason to settle for an inferior design; and also that's something that can/should be solved by the code generator (same as we do with cases like "whoops import happens to be a reserved word here let's append a _" or "I will represent properties as getters and setters").

@eladb @RomainMuller

[eladb]

Makes sense... I suspect the reason we didn't use interfaces back then was that jsii didn't support interfaces, so we tried to avoid them...

My only concern is that there's a difference in API capabilities between the "ref" object and the concrete object. This means that there will be potentially a few capabilities one can do with a NetworkLoadBalancer but can't do with INetworkLoadBalancer. The Ref semantic gave affordance to that (by implying that "this is not a real bucket"). I am curious whether the interfaces should be named INetworkLoadBalancerRef (hate it) to provide a similar distinction.

What do you think?

[rix0rrr]

Not sure what you mean. Isn't it the same distinction?

An INetworkLoadBalancer exposes those functions that consumers can expect from a load balancer "in the abstract", same as a NetworkLoadBalancerRef would expose. The only difference is that NetworkLoadBalancerRef will also impose (part of) an implementation, whereas INetworkLoadBalancer won't.

[eladb]

Yes, makes sense. I also really dislike the “Ref” terminology.

Let’s do it!