X2y: A data-serialization file transcoder

Hello everyone,

I recently released the first version of a small command line program, x2y, that
transcodes a few common data-serialization languages. You can turn a
toml file into a json or yaml file and vice versa or even an entire directory of these files if you so choose.

It is built atop of serde and the relevant language parsing libraries.

I would really appreciate some feedback on the project so far and any areas that
I can improve. The repo can be found here: GitHub - edneedham/x2y: A data-serialization file transcoder.

Thanks for your time,
Ed.

Instead of reading the entirety of the input into a big value, you should probably use serde_transcode for performing the conversion in a streaming fashion. That way, your tool would be usable with big inputs that don't easily fit in main memory.


Memory usage aside, there are many code smells in the transcoder module:

  • transcode() declares a result variable just to return it. That's unnecessary.
  • You are performing repetitive explicit matching over Results just to return early. You should instead be using the ? operator for propagating errors.
  • You are calling .to_string() on Strings. That's just unnecessary.

And elsewhere:

  • in app, you should be deriving a Clap Parser on a strongly-typed configuration type, instead of configuring everything manually and then re-parsing the dynamically-typed arguments when you actually need them.
  • in format, the Format enum doesn't implement Eq and Hash.
  • still in the same module, on line 30, you are embedding a Debug representation into an error message. Don't – errors should be Display, of which the output must be stable, unlike that of Debug.
  • in fs, you are removing the input files. I'm not sure that's great UX.
  • in the same module, you define trivial functions open_file, remove_file, etc., which are equivalent with .map_err(), and which are completely unnecessary anyway, since the From<io::Error> impl of X2YError means that you could just use the ? operator again to automatically convert an io::Error to an X2YError.
  • in lib.rs, the way you cfg out the test_helpers module in a roundabout way is weird. Why don't you simply put the #[cfg(test)] directly on a mod test_helpers; declaration?

I'm pretty sure there are deeper problems as well, but I didn't look further yet. You should probably be using Clippy for some automatic feedback.

2 Likes

Thank you very much for taking the time to go through my project and providing such a comprehensive list of ways to improve it. A lot of the improvements seem obvious now that you mention them and I look forward to making those changes.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.