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

Fundamental problem with dupliated keys - you can't get duplicated values #92

Closed
ghost opened this issue Oct 24, 2019 · 6 comments
Closed

Comments

@ghost
Copy link

ghost commented Oct 24, 2019

Hi,

I would like bring you attention to the fact that there is a fundamental problem with duplicated keys: you can't get duplicated values. It's not the fact that HJSON allows to create it. Being unable to receive them via parser and the fact that you always receive last value, is in my opinion a road to hell.

Image a config file which is 250+ lines:

  • there is a key named "ThroleLimit" at the beginning of the file with the value '9'
  • there is a another duplicated key after line 200 with the value '0'

Notice how easy is to mistype 9 with 0. When such config will be loaded into system, the system will break because of the ThroleLimit will be 0 and noone will know why because first key has value of 9. No one will fix the bug until they will magically find duplicated key.

That is very serious design issue in my opinion. It makes HJSON more inline with normal text files. There are cases when duplicate keys are useful but I think that for configuration files, it is not necessary. Config files are always represent the unique state of things and if not, it's not config file anymore, it's data for interpretation.

My further thoughts about this:

  • in ideal word, HJSON could change parser requirements and add requirements to return duplicate keys. But it will never 'fly' because it adds complexity and effort so developers will simply ignore such requirements and go back to 'use last key value'
  • such feature aka 'allow duplicate keys' is so dangerous that it should have it's own file format like 'dk' which stands for duplicate keys

My proposal:

  • Remove "duplicate keys" feature from HJSON specification
  • Create copy of the HJSON which will be called HJSONDK
  • then, for HJSON format, it won't matter if the exta values can be recived or not, or how to parse them
  • Every people who needs "duplicate keys" feature feature should be directed to the new HJSONDK format

That way, you can remove this feature without actually removing it from the people who really need it.

@ghost ghost changed the title Fundamental problem with dupliated keys - you can't get them Fundamental problem with dupliated keys - you can't get duplicated values Oct 24, 2019
@dqsully
Copy link
Member

dqsully commented Oct 24, 2019

To be honest I didn't even know this was a feature of Hjson. However, I think the best way to implement this change is just to introduce a parser flag. I don't think there is a good enough reason to split into another variant of the Hjson language just for one feature.

@3F
Copy link

3F commented Oct 24, 2019

Hmm, nice catched undeclared behavior. Looks like even current rfc draft does not mention any spec for duplicated keys.

My suggestion is to extend the representation of arrays as the Level-based Arrays.

Here's issue: #93

@sffc
Copy link

sffc commented Oct 24, 2019

Standard JSON (ECMA-404) has no rule against duplicate keys, and most implementations simply accept the last entry from a duplicated key. Hjson choosing to reject duplicate keys would make it stricter than ECMA-404, meaning valid JSON would no longer necessarily be valid Hjson. So in my opinion, this issue is a non-starter for that reason.

@sffc
Copy link

sffc commented Oct 24, 2019

To be clear, implementations could feel free to add a flag option to the parser that throws if it finds duplicated keys, but this should not be part of the core Hjson specification.

@ghost
Copy link
Author

ghost commented Nov 5, 2019

Standard JSON (ECMA-404) has no rule against duplicate keys, and most implementations simply accept the last entry from a duplicated key. Hjson choosing to reject duplicate keys would make it stricter than ECMA-404, meaning valid JSON would no longer necessarily be valid Hjson. So in my opinion, this issue is a non-starter for that reason.

I didn't know that JSON has no rule against duplicated keys. Now it looks like the whole foundation of (H)JSON is not good for configuration files. While this is sad, I don't think that anything can be changed for that matter.

To be honest I didn't even know this was a feature of Hjson. However, I think the best way to implement this change is just to introduce a parser flag. I don't think there is a good enough reason to split into another variant of the Hjson language just for one feature.

To be clear, implementations could feel free to add a flag option to the parser that throws if it finds duplicated keys, but this should not be part of the core Hjson specification.

The parser flag is not a valid solution for the problem which I described because there is a possibility that such flag will be missed/removed by mistake.

Now when I know that ECMA-404 has no rule against duplicated keys, I'm accepting the fact that (H)JSON must somehow follow this standard and even if the 'no duplicated keys' rule would be introduced, most parsers would simply ignore these requirements.

It seems to me that what I wish is a new config file standard which is based on JSON spec in terms of objects but doesn't need to follow it strictly. Thanks for the discussion.

@sffc
Copy link

sffc commented Nov 6, 2019

It seems to me that what I wish is a new config file standard which is based on JSON spec in terms of objects but doesn't need to follow it strictly. Thanks for the discussion.

https://xkcd.com/927/

I don't see duplicated keys as such a huge problem that it warrants yet another config file standard. Besides, duplicated keys are something difficult to express in an RFC. I think this is a problem easily solved by changing your Hjson parser implementation to have an option that makes it throw on duplicated keys.

@ghost ghost closed this as completed May 3, 2020
This issue was closed.
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

3 participants