search

chyh1990 / yaml-rust

A pure rust YAML implementation.
Apache License 2.0
601 stars 138 forks source link

About LinkedHashMap #154

Open KSXGitHub opened 4 years ago

KSXGitHub commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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.