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

About LinkedHashMap #154

Open
KSXGitHub opened this issue Apr 3, 2020 · 2 comments
Open

About LinkedHashMap #154

KSXGitHub opened this issue Apr 3, 2020 · 2 comments

Comments

@KSXGitHub
Copy link

KSXGitHub commented Apr 3, 2020

Problem

When I want to create a YAML text from a YAML dictionary, I must first create a Yaml::Hash value. In order to do that, I have to also use linked_hash_map crate. This touching yaml-rust's implementation detail and pray that it does not switch to something else (or having an incompatible version).

Proposals

Do one of the following:

  1. Don't use linked_hash_map::LinkedHashMap directly, instead, create a wrapper type and expose it.

  2. Alternatively, still use linked_hash_map::LinkedHashMap, but also expose it by pub use linked_hash_map. This action requires mininal effort, but this would also mean that yaml-rust's semantic versioning now depends on linked-hash-map as well.


P.S. This issue need a better title. Any suggestion?

@chyh1990
Copy link
Owner

chyh1990 commented Jun 2, 2020

I think (theoretically) it is better to wrap internal map (e.g. LinkedHashMap) with a proxy layer to hide its implementation.

Also make it easier to switch to other map implementation, without breaking map APIs like #157

@KSXGitHub
Copy link
Author

@chyh1990 Either way is fine to me. This is a choice for you — the maintainer to make. In my opinion, the first choice would make switching between implementations easier, while the second choice is not a breaking change.

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