sbcgua / ajson

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

Change converter/mapper application moment #112

Closed sbcgua closed 1 year ago

sbcgua commented 2 years ago

followup from #110, to think and maybe adjust the behavior

at the parsing step the json is parsed and the mapper is assigned after. Maybe it's wrong. Maybe I'd expect the camel case converted into snake (abap-like) - field_data

the conversion should happen only on in/out ? Like parsing/serialization ? And the set should keep the name as is ?

CC: @jrodriguez-rc @albertmink

jrodriguez-rc commented 2 years ago

In the context of #110 , we are talking about JSON to JSON conversion?

It is not expected to serialize JSON with ABAP format (camel to underscore, for example)

If JSON to JSON will be supported then a new method will be created in the mappers to transform from JSON (this is the source format stored in AJSON instance) to JSON, and it will be called on the stringfy method

sbcgua commented 2 years ago

I guess we are talking about different things. What I mean, I think there are 2 basic cases:

1) I have a json with CC (camel case). And I'm fine with it. I'm going to access certain nodes by full path or members or slice or whatever. I don't need conversion.

2) I have a json in CC with a predictable structure and I'm going to transfer it to abap structures/tables (or from abap structures). I don't care how it stores the data internally, just want a convenient short way to operate with it. And I want that the string representation is camel cased.

In the 2nd case I think it cleaner to operate with snake case while in abap, so while I'm getting/setting things, so always. The only moment I want to see the CC is (de)serialization. Thus the conversion would be relevant only for parse and stringify. Moreover, it is unnecessary to pass the converter to the constructor, but just to parse/stringify methods. Architecture-wise it will also locate the conversion related code in fewer places, which is good.

Also, if I understand you correctly, maybe there is a method convert somewhere to force-apply the conversion to a json instance (or maybe to a new json instance copy).

sbcgua commented 2 years ago

Just thoughts in the background. Maybe we even don't need the conversion at parse/serialize time ... and it should be a create_from option. Just 1 place. It would simplify the architecture even more.

orig_json = zcl_ajson=>parse( ... ).
adapted_json = zcl_ajson=>create_from(
  ii_json = orig_json
  ii_filter = ... " this is already there ! which adds to the idea that it is THE right place for these things !
  ii_field_name_mapper = ... " mapper here
)

The "library" would then have:

So could be

json = zcl_ajson=>create_from(
  ii_json = zcl_ajson=>parse( ... )
  ii_field_name_mapper = zcl_ajson_mapping=>from_camel_case( )
)

The zif_ajson_mapping would then have only

  types:
    begin of ty_mapping_field, " or better ty_rename
      from type string,
      to type string,
    end of ty_mapping_field,
  methods map_field_name
    importing
      !iv_path         type string
      !iv_name         type string
    returning
      value(rv_result) type string.
sbcgua commented 1 year ago

Kind of done, see:

Will close in a while.