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

Clean up format #18

Closed
jasongitmail opened this issue Jan 23, 2024 · 6 comments
Closed

Clean up format #18

jasongitmail opened this issue Jan 23, 2024 · 6 comments
Labels
Change: major [Issue / PR] describes a breaking change of any kind Domain: main [Issue / PR] describes change in the functionality, its optimization Rejected: by-design [Issue / PR] describes unindended changes in functionality Type: improvement [Issue / PR] addresses lack of a functionality or an open possibility of enhancement

Comments

@jasongitmail
Copy link

jasongitmail commented Jan 23, 2024

I noticed the data at @iso4217/json/data.json is in an unusual format. I assume it's what is generated by the XML converter.

Given that this data is an a known format, it'd be more useful to transform this a cleaner format.

Existing:

...
        {
          "$name": "CcyNtry",
          "$attr": {},
          "$data": [
            {
              "$name": "CtryNm",
              "$attr": {},
              "$data": "SOLOMON ISLANDS"
            },
            {
              "$name": "CcyNm",
              "$attr": {},
              "$data": "Solomon Islands Dollar"
            },
            {
              "$name": "Ccy",
              "$attr": {},
              "$data": "SBD"
            },
            {
              "$name": "CcyNbr",
              "$attr": {},
              "$data": 90
            },
            {
              "$name": "CcyMnrUnts",
              "$attr": {},
              "$data": 2
            }
          ]
        },
...

Suggested, example:

        {
          "countryName": "SOLOMON ISLANDS",
          "currencyName": "Solomon Islands Dollar",
          "currencyCode": "SBD",
          "currencyNumber": 90,
          "currencyMinorUnits": 2
        },

or

        {
          "countryName": "SOLOMON ISLANDS",
          "name": "Solomon Islands Dollar",
          "code": "SBD",
          "number": 90,
          "minorUnits": 2
        },
@parzhitsky
Copy link
Member

Thanks for the suggestion! There are several problems with it.

  1. No, this is not the result of any third-party XML converter. Although I use an npm package to read the XML file, that result is considered to be intermediary and is then converted (again) to the published format (see Get rid of intermediate JSON #1).

  2. The published format addresses the fact that while a JSON value can have child values of only one kind (nested value), an XML node can have child values of two kinds: nested nodes and attributes. There is no alternative to attributes in JSON, and I want to keep nested nodes distinct from attributes. I actually plan to make it even more verbose (see Preserve raw content in $text node #2).

  3. This would be a backwards-incompatible change. Due to the choice of versioning strategy (see Readme) I can't make any backwards-incompatible changes, only minor and patch releases.

Consider using a library that uses JSONPath to extract necessary data from the currency JSON file. I may add this capability in my package too but this needs to be done carefully.

@parzhitsky parzhitsky added Change: major [Issue / PR] describes a breaking change of any kind Domain: main [Issue / PR] describes change in the functionality, its optimization Rejected: by-design [Issue / PR] describes unindended changes in functionality Type: improvement [Issue / PR] addresses lack of a functionality or an open possibility of enhancement labels Jan 23, 2024
@parzhitsky parzhitsky closed this as not planned Won't fix, can't repro, duplicate, stale Jan 23, 2024
@jasongitmail
Copy link
Author

jasongitmail commented Jan 24, 2024

Interesting. Thanks for the explanation.

I suppose I'd assume that there is a known set of properties for any given ISO data standard, allowing it to be transformed arbitrarily and easily into a more user-friendly JSON format.

And something like JSONPath would only be needed if you were trying to generalize the code to multiple standards and didn't want to research the nuances of multiple standards or wanted to maintain flexibility.

But I'm sure you've considered this.

@parzhitsky
Copy link
Member

That makes sense, but the backcompat issue basically kills a ton of improvement potential

@jasongitmail
Copy link
Author

major version bump?

@parzhitsky
Copy link
Member

Yeah, because the format is different. A codebase that assumes current shape of the JSON would start producing errors with the new format — hence the major bump.

@jasongitmail
Copy link
Author

👍 Yeah. Was a suggestion. Not locked into the current format due to backward compat considerations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Change: major [Issue / PR] describes a breaking change of any kind Domain: main [Issue / PR] describes change in the functionality, its optimization Rejected: by-design [Issue / PR] describes unindended changes in functionality Type: improvement [Issue / PR] addresses lack of a functionality or an open possibility of enhancement
Projects
None yet
Development

No branches or pull requests

2 participants