1

Is it worth implementing interfaces, or using the getThis() trick, or even static factory methods to future proof against the need to extend this DSL?

The current design is a pair of thin fluent-method-chained-DSLs that mix the "traditional" and "enhanced" builder patterns. That's just a fancy way to say some chained methods are optional and not ordered while others are required and ordered.

  • If the method just sets a value the reasons for the required/optional difference comes down to if there is or isn't a good-default value. A good default value means setting it is optional. No good-default means this is a required step. If it's a required step it's nice to do required steps in a predictable order.

  • If the method performs some instructions the required/optional difference comes down to if the instructions can be skipped and still give a useful result.

That's the reasoning behind mixing the two styles. It sounds complicated but done well it allows all the rules for using the fluent interface to not only be enforced at compile time but be made obvious to the user in the IDE's intellisense.

That means to use this code you wouldn't need to look at this design document:

//Paste this BNF in the Edit Grammer tab of http://bottlecaps.de/rr/ui

Legend ::= Legend

Legend ::= 'Method' (    ReturnType -'') 

Legend ::= 'OrClassDotMethod' (    MayChangeOrNot -'')

Legend ::= 'ReallyItsTheStuffYouType' ( ButItIsntWhatYouTypicallyType -'') 

ServiceThis ::= 'ServiceThis._formatRequest(request)' ( ( ServiceFormatRequest -'') )

FormatService ::= 
    ( 
        (ServiceFormatRequest -'')  
           ( '._withThisFormatService(formatService.class)'
           | 
           )
    )+ 
    ( '._serviceTheRequest();' (    File -'') )

ServiceThis ::= 'ServiceThis._generationRequest(request)' ( ( ServiceGenRequest -'') )

GenService ::= 
    ( 
        (ServiceGenRequest -'')  
           ( '._withThisGenService(genService.class)'
           | '._withRandomFile(relitivePath, fileName)' 
           | '._withRandom()'
              ('._of(listOfHex)'
              |'._of(hexArray)' 
              |'._of(hex)' 
              )
           | 
           )
    )+ 
    ( '._serviceTheRequest();' (    Fetchable -'') )

Fetch ::= 'FetchResultOf._generationRequest(request)' (    Fetchable -'')

Fetchable ::= (Fetchable -'')
    ( '._editionFetchFirstOnly()' (OneEditionFetchElement -'' )
    | '._editionFetchAll()' (MultiEditionFetchElement -'') )

MultiEditionFetchElement ::= (MultiEditionFetchElement -'')
    ( '._elementFetchFirstOnly()' (AllAndOne -'') 
    | '._elementFetchAll()' (AllAndAll -'')
    | '._asRawStorableObject()' (': List<StorableObject> '-'')
    | '._asCheckedUpCastToCollectionOf(Some.class)' (': List<Collection<Some.class>> '-'') )

OneEditionFetchElement ::= (OneEditionFetchElement -'')
    ( '._elementFetchFirstOnly()' (OneAndOne -'')
    | '._elementFetchAll()' (OneAndAll -'')
    | '._asRawStorableObject()' (StorableObject -'')
    | '._asCheckedUpCastToCollectionOf(Some.class)' (': Collection<Some.class> '-'') 
    )

OneAndOne ::= (OneAndOne -'')    
   ( '._asKey();' (Key -'')
   | '._asObject();' (Object -'')
   | '._asString();' (String -'') 
   | '._asBitString();' (BitString -'')
   | '._asBigInteger();' (BigInteger -'')
   | '._as(Some.class);' (Some -'')
   | '._asCollectionOf(Some.class);' ( ': Collection<Some> '-'')
   | '._asMapOf(Some.class, SomeOther.class);' (': Map<Some, SomeOther> '-'')
   )

OneAndAll ::= (OneAndAll -'')
   ( '._asKey();' (': List<Key>'-'')
   | '._asObject();' (': List<Object> '-'')
   | '._asString();' (': List<String> '-'')
   | '._asBitString();' (': List<BitString> '-'') 
   | '._asBigInteger();' (': List<BigInteger> '-'')
   | '._as(Some.class);' (': List<Some> '-'')
   | '._asCollectionOf(Some.class);' (': List<Collection<Some>> '-'')
   | '._asMapOf(Some.class, SomeOther.class);' (': List<Map<Some, SomeOther>> '-'')
   )

AllAndAll ::= (AllAndAll -'')
   ( '._asKey();' (': List<List<Key>>'-'')
   | '._asObject();' (': List<List<Object> '-'')
   | '._asString();' (': List<List<String>> '-'') 
   | '._asBitString();' (': List<BitString> '-'') 
   | '._asBigInteger();' (': List<List<BigInteger>> '-'')
   | '._as(Some.class);' (': List<List<Some>> '-'')
   | '._asCollectionOf(Some.class);' (': List<List<Collection<Some>>> '-'')
   | '._asMapOf(Some.class, SomeOther.class);' (': List<List<Map<Some, SomeOther>>> '-'')
   )

Hope you'll forgive me for hacking bottlecaps.de's BNF to re-purpose their expression symbol to show type info. Ignore the -'' since that's just the hack.

Or remove them with this trick:

Add this after meta tags and before ::-moz-selection at the top

    p
    {
      display: none;
    }
    svg
    {
        padding-top: 25px;
        padding-right: 50px;
        padding-bottom: 25px;
        padding-left: 50px;
    }

replace with empty string:
' - ''
- ''

replace with &nbsp;&nbsp;&nbsp;
': 

Example 1

String helloWorld = ServiceThis
    ._request(request)
    ._withCleanDBSchema()
    ._withLog(log)
    ._doServiceRequest()
    ._fetchOnlyFirstRank()
    ._fetchOnlyFirstElement()
    ._asString();

Example 2

ServiceThis
    ._request(request)
    ._withCleanDBSchema()
    ._withLog(log)
    ._doServiceRequest();

String helloWorld = FetchResultOf
    ._request(request)
    ._fetchOnlyFirstRank()
    ._fetchOnlyFirstElement()
    ._asString();

These two examples do exactly the same thing but they show off that it is possible to separate servicing the request and fetching its results.

This code is intended to alleviate two problems: Our system needs to be prepped before servicing a request and the results are coming back in a RAW container and, depending on the request, may or may not be 2 dimensional.

This code will abstract away system details and return properly typed/generic results. This will let us focus on the service under test.

The Java code works at my desk but it needs to work for everyone. Our environment is not friendly to refactoring. We don't do continuous integration. That means once this gets used it's about the same as if it's been published to a 3rd party.

However, this is test code, not operational code. So I don't want to over engineer it and annoy people. This makes me question if I should do either of the following:

  • (1) Provide interfaces. I'm only creating one version for now. People find interfaces distracting when tracking down what threw an exception. But this will be used by a lot of test code that will not be possible to refactor.
  • (2) Use the getThis() trick to allow some future version to return it's exact types even from methods it doesn't override. Programming to a fixed interface doesn't account for the occasional need to add methods that didn't exist before. But really how practical is reusing code when the reusing code ends up just as complicated?

(1)

For example, say later I want to add a ._fetchDiagonally() method. I've used static factory methods to start both of these method chains. They both return instances of classes not defined within them. I can simply sneak into the factories and have them construct completely different classes. As long as all the methods are still supported all dependent code should still work as long as clients haven't done something silly like this:

ServiceThisDefaultImpl srdi = ServiceThis.request(request);
srdi.doServiceRequest();

Arg! Now I can't change my static factories. Some client code is insisting that it get ServiceThisDefaultImpl. This problem cascades down every step of the lock stepped fetch.

Now I can stop this by implementing and returning interfaces all down the chain. Then you'd have to do a cast to force ServiceThisDefaultImpl to matter. And at that point I figure it's your own fault.

However, if I do implement interfaces for each step method (I can do it all in one file by nesting them) I can swap implementations sure enough but the second implementation ends up being almost exactly as complicated as the default one. I've already factored out the work behind the methods into individual classes so their work ends up being one liners.

As such I'm not sure it wouldn't be easier to forget about implementing interfaces and instead let a second implementation start over from scratch since these two fluent DSLs are just a thin facade. That way, I wouldn't care what weird things clients do since I'm not touching their code.

(2)

What does the getThis() trick offer? Is it incompatible with the idea of using interfaces? Using abstract classes rather than interfaces doesn't seem like it makes much difference when there is no implementation at the higher level.

I'd like to be able to sneak a new method in that hadn't existed before while the old clients are none the wiser. Am I correct in thinking getThis() allows this? Unlike programming to an interface I won't be locked down to that first set of methods since I'm actually going to return the low level types not the high level abstractions.

The example makes clear that this would work for a "traditional" builder (like ServiceThis) but is it feasible for an "Enhanced" builder (like fetch)?

And again, is it worth it? I've no idea what code I'd put up at the abstract level. So I can't really see what it's getting me over just starting over with a different fluent DSL interface. (Which would mean starting with completely different static factories which makes me wonder why I even need static factories).

If (1), (2) and the factories really are over engineering I'm suspecting the reason is that the thin fluent DSL is in fact an "interface", or at least an abstraction, all it's own and doesn't need the extra help. Or am I missing something?

migrated from codereview.stackexchange.com Sep 16 '14 at 7:23

This question came from our site for peer programmer code reviews.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.