Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

option to ignore initial values? #71

Closed
larshp opened this issue Sep 6, 2021 · 10 comments
Closed

option to ignore initial values? #71

larshp opened this issue Sep 6, 2021 · 10 comments

Comments

@larshp
Copy link
Collaborator

larshp commented Sep 6, 2021

I have the following code,

DATA intf TYPE zif_aff_intf_v1=>ty_main.
DATA li_ajson TYPE REF TO zif_abapgit_ajson.
intf-header-description = 'hello world'.
li_ajson = zcl_abapgit_ajson=>create_empty( ).
li_ajson->keep_item_order( ).
li_ajson->set(
  iv_path = '/'
  iv_val  = intf ).
DATA(json) = li_ajson->stringify( 2 ).
BREAK-POINT.

it creates the following json,

{
  "schema": "",
  "header": {
    "description": "hello world",
    "original_language": "",
    "abap_language_version": ""
  },
  "category": "00",
  "proxy": false,
  "descriptions": {
    "types": [],
    "attributes": [],
    "events": [],
    "methods": []
  }
}

I'd like to ignore initial fields, ie. expected outcome is

{
  "header": {
    "description": "hello world"
  }
}

any suggestions/thoughts on implementing this?

@sbcgua
Copy link
Owner

sbcgua commented Sep 6, 2021

Should not be difficult to implement.
There is ignore_empty flag in set. However it works for the whole value. I'm wondering if it is semantically correct to extend the same flag to components of a structure ... or it should be a separate flag ? Currently gravitating towards the former.

@larshp
Copy link
Collaborator Author

larshp commented Sep 6, 2021

also, I'm considering extending the custom mapping class/interface to also be able to handle values, as I'd like to keep some initial values

@larshp
Copy link
Collaborator Author

larshp commented Sep 7, 2021

perhaps something like adding this method to the custom mapping interface

methods to_json_skip_value
  importing 
    io_type type ref to cl_abap_typedescr
    is_node type ty_node,
  returning 
    value(rv_skip) type abap_bool.

if it returns rv_skip = abap_true, then skip the value, else as normal

@sbcgua sbcgua mentioned this issue Sep 8, 2021
@sbcgua
Copy link
Owner

sbcgua commented Sep 8, 2021

Separated the last comment into an issue #72 (good idea by the way !)

Looking into ignore_empty ... there is a stupid issue on the way ... ignore_empty = true by default :) So current semantics can be used somewhere already. Does it make sense that it is true by default? and thus:

  • ajson skips direct empty values
  • and also skips all deep empty attrs of structures? And structures in structures ? And structures in tables ? (but not empty table items, unless the whole table is empty) ... hmmm

UPD: Otherwise we need a separate flag for deep skipping .. or just implement #72 as the only way to do it

@larshp
Copy link
Collaborator Author

larshp commented Sep 8, 2021

having ignore_empty on by default, might be a breaking change, not for ABAP, but if there are consumers of the json in languages that respect undefined

but anyhow, I'm all for breaking stuff #keepTheCoreMoving

@sbcgua
Copy link
Owner

sbcgua commented Sep 8, 2021

In the meantime I think it is "just implement #72 as the only way to do it"... (in progress)

@larshp
Copy link
Collaborator Author

larshp commented Sep 10, 2021

yea, so close this issue?

@sbcgua
Copy link
Owner

sbcgua commented Sep 10, 2021

Not yet, let's keep it for clarity, the task is a bit more difficult than I thought

@sbcgua
Copy link
Owner

sbcgua commented Oct 10, 2021

@larshp reimagining the feature ... what is the final purpose of the feature?
Is it a json tree that skips certain values on set ?

json state: { a: 1, b: 2 }
li_json-set( iv_path = '/filtered/node' iv_value = xxx )
json state: { a: 1, b: 2 }

Or is it a serialized json without certain nodes ?

json state: { a: 1, b: 2 }
li_json-set( iv_path = '/filtered/node' iv_value = xxx )
json state: { a: 1, b: 2, filtered: { node: xxx } }
li_json->serialize( ) => "{ a: 1, b: 2 }"

I believe it is the 2nd

The first feature is adding a lot of complexity when setting values. While the 2nd is separatable in an independent logic

...
json state: { a: 1, b: 2, filtered: { node: xxx } }
li_json2 = li_json->filter( li_filter_instance )
json2 state: { a: 1, b: 2 }
li_json2->serialize( ) => "{ a: 1, b: 2 }"

Does it make sense ?

Why complexity - because of deep nature of json. E.g. the above example adds '/filtered/node' where filtered does not exit yet. So ! Both '/filtered/node' and '/filtered' must be skipped. Or if '/filtered' already exist then it should stay. Not only will it make set mote complex but also slower.

@sbcgua
Copy link
Owner

sbcgua commented Aug 1, 2022

The filter was implemented, can this be closed ?

@larshp larshp closed this as completed Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants