Please, make a code review

I’m trying to write my first small project on rust. This is a small library. I would like to understand how correct and idiomatic code I am writing. I would be grateful if someone makes a code review and gives his opinion, makes comments. The source code is very small and simple. Thank you in advance for your help.

Your lib.rs only contains pub mod vfs; (without a newline in the end!). Your library documention should go in there (see lazy_static for an example.


Use cargo fmt to format your code according to the community guidlines or provide your own rustfmt.toml file in the root of the project, so that everybody can use rustfmt to format it according to your style.


There are examples missing, e.g. how to use which struct and method. The library documentation would be a perfect place for it.

I somtimes see

if a {
    self.foo = 3;
} else {
    self.foo = 5;
}

Rewrite it to:

self.foo = if a { 3 } else { 5 };

Dont use C-declaration of variables, e.g. let pb: PathBuf;, but instead use the “technique” above.


Instead of panicking in tests, you could return a Result<(), _> from the test method (Playground) to test if a certain test has a Err Result or use a match, but panicking is not a good solution (IMHO!).


Although its possible in Rust 2018, I would not recommend to have a vfs.rs and a vfs folder in the same directory. Your whole project architecture is not very well structured. I would put most of the content you have in your vfs.rs into your lib.rs and the error if needed into error.rs into the same directory.
If you want subfolders, go with the old layout vfs/mod.rs vfs/error.rs and then you can do use vfs for everything that is in your mod.rs.

1 Like

Thank you very mach! Your answer is more then my lib ))

What’s your reason for this suggestion? Even that it has the disadvantage of having not all code of a module in one folder, I think it has some main advantages for ergonomics:

  1. You can easily add submodules: With the “2018-layout” you don’t have to refactor your projects layout. Just add a folder with the correct name and place the new files in it. With the “mod.rs-layout” you would first need to create a new folder, rename your original file into mod.rs and move ot into the folder (which also breaks the tracking in git).
  2. Files can be identified by name: I use VS code and its way more ergonomic to do ctrl+p and type specific_mod.rs rather than typing mod.rs and get a number of suggestion that are only distinguishable by there path.

It’s my personal opinion. Maybe because I learnt rust from the pre-2018 era and it was not possible there and now I stick to that convention.
Your points are valid, I’m not saying anything against them. Thank you for bringing them in.

2 Likes

It’s interesting that your development environment makes the new style more ergonomic. I use Vim in the command line, and rarely have an instance of Vim open for a long duration, so I find the old style much more ergonomic for tab-completing from the shell vim src/v<tab>m<tab> and vim src/v<tab>e<tab> is better than vim src/v<tab>.rs and vim src/v<tab>/e<tab> because the completion takes care of all the punctuation characters for me; I just have to think about the names of the files.

2 Likes

What about such long constructions: path.as_ref().to_str().unwrap() - is this normal?
Or is it a sign of an incorrectly written environment?

You could instead store a &Path in PathNotBelongsError and use Path.display to format it in the Display impl.

On another note: Errors only need to implement Display, the description method is deprecated.

1 Like

I went ahead and made a more detailed review: https://gist.github.com/bash/69ecd6656cc0f4b804185b5cdac2a243

1 Like

Thank you for such a informative review. I found a lot of useful things for myself.

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.