Reusable server-side processing with flexibility

Reusable server-side processing with flexibility

Restful Web ServicesRestful Web Services Posts: 202Questions: 49Answers: 2
edited April 2018 in Feature requests

It is my understanding that we should always be attempting to follow the DRY (do not repeat yourself) principle of software development. We should also, where possible, only process and serve data which is actually required by the specific request. At present given the following basic example I am unable to see how I can follow these principles when creating my server-side processing code.

Basic example
I have three tables; users (parent table), transactions (one-to-many join using the link table) & link_table. The main use of the users table will be to allow CRUD operations for my list of users and in general I don't want to see all the linked child items from the transactions table. However, occasionally I will want to drill down and see all the transactions. I still want to see the transactions in the context of my users so I need all the information from the users table plus the data from my transactions table. On the client side I can achieve this by simply changing the visibility of the column containing my mJoin transactions table data. However, on the server side this mJoin data is processed and loaded into my JSON response on every request, irrespective of whether I actually want to use the mJoin data on the client side. So memory is being used and data is being sent even though I don't specifically need it for that request. This unnecessary overhead could be slowing down my user experience, especially if the mJoin table holds thousands of records, so I copy my table.users.php file and remove the mJoin from the copied file. I can now load whichever server side file I need depending on what I want to show the user. However, and this is the crux, I now have two almost identical files which essentially do almost identical functions and I have to maintain both.

To my mind this seems potentially to go against the DRY principles of good software development. Would an option based on these principles be to have the ability to turn on and off which fields require processing on a per request basis. I.e. the equivalent of showing and hiding a column on the client side. Now I appreciate this is potentially more complex on the server-side but I was just wondering if anyone had any thoughts on this?

Answers

  • colincolin Posts: 15,112Questions: 1Answers: 2,583

    Hi @Restful Web Services ,

    Yep, where possible DRY is a good principle to follow, but I also like KISS (Keep It Simple, Stupid!). If adding complication to avoid repetition makes the code convoluted, then it's not really worthwhile IMO. Where those boundaries lie is hard to say without greater knowledge of the code or a feel of the system.

    Not much help, but my 2c worth!

    Cheers,

    Colin

  • tangerinetangerine Posts: 3,342Questions: 35Answers: 394

    @Restful Web Services - Colin is right.
    The various "principles of software development" are not engraved on tablets of stone. DRY has probaly cost me as much time as it has saved. Hindsight shows that special cases or anomalies may keep turning up, and I am then faced with cramming a variety of conditionals into my old DRY code, or splitting it into separate functions in line with the new circumstances

    Also, another useful "principle" is "Don't Optimize Too Early" - which you may be doing here.

    I now have two almost identical files which essentially do almost identical functions and I have to maintain both.

    Should that be two classes inheriting from a common base class?

  • Restful Web ServicesRestful Web Services Posts: 202Questions: 49Answers: 2

    Thanks for your responses. Just to be clear with respect to Tangerine this is only a hypothetical discussion, I am not actually coding this example. I am just interested in whether my thought has any validity.

    I 100% agree with using the KISS principle and for me the crux here is whether it is possible to implement some sort of solution without introducing unnecessary complexity. I.e. if adding various conditional functions ultimately makes the single file more time consuming to maintain and more inefficient to run then ultimately it has been a pointless exercise. However, if you can introduce the additional functionality whilst mainting DRY and KISS then maybe there could be some benefit.

    A very basic example, again just hypothetical, if this was the client side code:

    }
    data : "user.email",
    visible: false,
    {
    

    And this is the server side code:

    DataTables\Editor\Field::inst( 'user.email' )
    ->validator( DataTables\Editor\Validate::notEmpty(
     DataTables\Editor\ValidateOptions::inst()->message( 'Email address cannot be empty' )
    ))   
    ->validator( DataTables\Editor\Validate::email(
    DataTables\Editor\ValidateOptions::inst()->message( 'Email must be a valid email address' )
    )),  
    

    If we could introduce a very simple process type flag to indicate if this field should be processed and this flag was, for example, powered by the visible value.

    DataTables\Editor\Field::inst( 'user.email' )
    ->validator( DataTables\Editor\Validate::notEmpty(
     DataTables\Editor\ValidateOptions::inst()->message( 'Email address cannot be empty' )
    ))   
    ->validator( DataTables\Editor\Validate::email(
    DataTables\Editor\ValidateOptions::inst()->message( 'Email must be a valid email address' )
    ))
    ->process( $visible == true ? "proceed" : "skip" ),
    

    If the column is visible on the client side then include in the PHP process and if not skip processing for this field.

    A question for tangerine, when you say,

    Should that be two classes inheriting from a common base class?

    If you have time could you please elaborate a little on what you mean exactly?

    Thanks

  • allanallan Posts: 61,446Questions: 1Answers: 10,054 Site admin

    The key here is to remember that you can break the chain! You don't need to use:

    Editor::inst( ... )
      ->fields( ... )
      ->process( ... )
      ->json();
    

    You could equally do:

    $editor = Editor::inst( ... )
    $editor->fields( ... )
    $editor->process( ... )
    $editor->json();
    

    From there it is just a step to add the mJoin based on some logic condition sent form the client-side:

    $editor = Editor::inst( ... )
    $editor->fields( ... )
    
    if ( ... ) {
      $editor->mJoin( ... )
    }
    
    $editor->process( ... )
    $editor->json();
    

    What @tangerine was suggesting was basically that, but rather than using a procedural style of data as I have in the above little examples, have a class which you can make new instances of and extend with calls to other Editor methods as you require. From a KISS point of view the above code is a bit easier, but equally, OO style code can be easier to maintain long term. That's personal preference I'd say.

    Another option would be to load the relational data for each user on demand, as shown in this example.

    Allan

  • Restful Web ServicesRestful Web Services Posts: 202Questions: 49Answers: 2

    Hi Allan,

    Thanks for your answer. I was not aware that you could actually break the chain, although I am sure it states it in the documentation. That certainly does open up the possibility to introduce some additional logic in a potentially concise manner.

    Thanks for the suggestion.

    Chris

  • allanallan Posts: 61,446Questions: 1Answers: 10,054 Site admin

    If its in the documentation it is implicit rather than explicit, since that's a programming style rather than something that is specific to the Editor libraries. I like chaining as it can keep code clean and tidy, but it can also be useful to mix them together - e.g.:

    $editor = Editor::inst( ... )
      ->fields( ... )
     
    if ( ... ) {
      $editor->mJoin( ... )
    }
     
    $editor
      ->process( ... )
      ->json();
    

    Again, not something I would document as that's part of the PHP language, but useful to know all the same.

    Allan

This discussion has been closed.