sbcgua / ajson

Yet another json parser serializer for ABAP
MIT License
49 stars 15 forks source link

performance question - set & filter in one method call? #173

Closed oblomov-dev closed 5 months ago

oblomov-dev commented 5 months ago

Hello everyone,

It's fun to use ajson! Just a small question.

I have in mind to add structures and tables to ajson, but to keep the payload small, i want to delete all initial values.

Therefore i first add the value and then do a filtering. example 1 - simple :

    DATA:
      BEGIN OF ls_chart,
        int     TYPE i,
        type    TYPE string,
        options TYPE string VALUE `ABC`,
      END OF ls_chart.

    DATA(ajson2) = zcl_ajson=>create_empty( ).
    ajson2->set( iv_path = '/' iv_val = ls_chart ).

    "{"int":0,"options":"ABC","type":""}

    DATA(ajson2_filtered) = ajson2->filter( zcl_ajson_filter_lib=>create_empty_filter( ) ).
    "{"int":0,"options":"ABC"} <- needed result

This works fine. But as far as i can see this, we need two loops. One loop for adding and another one for filtering.

example2 - large tables:

    TYPES ty_chart LIKE ls_chart.
    DATA lt_chart TYPE STANDARD TABLE OF ty_chart.
    DO 10000 TIMES.
      INSERT ls_chart INTO TABLE lt_chart.
    ENDDO.

    DATA(ajson3) = zcl_ajson=>create_empty( ).
    ajson3->set( iv_path = '/' iv_val = lt_chart ).
    DATA(ajson3_filtered) = ajson3->filter( zcl_ajson_filter_lib=>create_empty_filter( ) ).

It's still fast but we do two loops and when adding more columns or rows it will unnecessarily cost more runtime.

Am I using ajson right in this situation or did i overlook something?

Looking for something like:

ajson->set_w_filter( my_filter , my_data )
mbtools commented 5 months ago

in general, set will ignore empty/initial value. In your case, the structure is not initial, only some fields are. you could loop of the fields of the structure and set them individually.

or we might need a new method set_structure where iv_ignore_empty works field-by-field.

oblomov-dev commented 5 months ago

Thank you for the information!

yes something like set_deep where you can import structures, tables and all that would be useful too.

But i think also the iv_ignore_empty is not flexible enough, because in certain situations you want to decide with the mapping more in detail how and if something is written into the json. maybe my example was a little bit misleading. In the mentioned situation i don't now the exact mapper, it is set from the outside and i just use it for the json creation.

better example:

    LOOP AT t_attri REFERENCE INTO DATA(lr_attri).

      ASSIGN (lr_attri->name_abap) TO FIELD-SYMBOL(<attribute>).

      ajson->set( iv_ignore_empty = abap_false iv_path = `/` iv_val = lr_attri->name_front ).
      ajson = ajson->filter( lr_attri->custom_filter ).

      ajson_result->set( iv_path = `/` && lr_attri->name_frontend iv_val = ajson ).
    ENDLOOP.

For example the mapping can be done beforehand, which is great because i can decide before importing for values/strucs/tabs how i want to have the output of the name and value in the target json. But the filtering can not be done beforehand, means i can not decide before importing if components of structures and fields of tables are accepted or not. i always need to do two loops for this.

Again, ajson is great and its completely fun to integrate it to abap2UI5! The API and coding is so straightforward and i am annoyed using so long other json mappers. Just asking for small improvements which could fit into the concept and would help me in this specific situation - its all about abap class attributes (which are set dynamically as well as their mapper+filter) mapping to JSON (which becomes a UI5 model at the frontend) and vice versa.

mbtools commented 5 months ago

The project is open for PRs. Create an issue with a unit test that show how a new method would be used. That's the quickest way to assess if it fits the project.

oblomov-dev commented 5 months ago

ok great, just wanted to make sure first that i don't overlook a functionality. I will come back with a PR.

sbcgua commented 5 months ago

The set with ignore_empty is indeed rather superficial at the moment. It just checks the upmost value, not deeper structure elements. This can potentially be improved. But! What is empty? In the above example 0 should be eliminated as well, because it is initial value.

A very similar "problem" in java script: "", 0, false, ... and also null and undefined - all are empty (!x === true). Yet null and undefined are specially empty! And thus there are a lot of edge cases.

Let's define the goal clearer.

sbcgua commented 5 months ago

But certainly not

ajson->set_w_filter( my_filter , my_data )

Maybe something like

ajson->bind_input_filter( filter )
ajson->set( ... usual way ... )
" while in the background ...
   if input_filter is bound and input_filter->keep_node( ... ) = true
     append_node
   endif

But I'm not 100% sure that it would be much faster. calls and ifs are also overheads. So needs to be tested thoroughly

oblomov-dev commented 5 months ago

thank you for checking this!

as i said in the other issue, maybe its good when i come back with a more realisitc example and then you can check it. i think also both issues are similar, one does json->abap the other does abap->json but with the same requirements, so if APIs are changed it make sense to take both directions at once into account. i will try to do a coherent example and then we can see.

two remarks:

oblomov-dev commented 5 months ago

I close this issue, concerning API adjustments for performance, its too similar to this issue: https://github.com/sbcgua/ajson/issues/174