
Ask HN: Rust experts – please advise - gkuk
Hi HackerNews community,<p>This is my first post here so please forgive me if below looks silly.<p>A few weeks back I have written a tool in rust. I am on holiday now and have some time to add improvements. Since I&#x27;m not expert in this language I&#x27;d love to get some feedback on where I can improve the code to make it more idiomatic and generally better. I use this tool on my servers to filter away all logs I know can be ignored so I can focus on logs that potentially require my attention.<p>https:&#x2F;&#x2F;github.com&#x2F;grzegorzk&#x2F;logmap
======
steveklabnik
A few quick things. Posting to users.rust-lang.org will probably get you more
comments than here.

Did you run clippy on this code? Rustfmt? those are two easy things.

#![allow(dead_code)] is pretty suspicious...

I would use format! here
[https://github.com/grzegorzk/logmap/blob/master/src/logmap.r...](https://github.com/grzegorzk/logmap/blob/master/src/logmap.rs#L50-L59)

[https://github.com/grzegorzk/logmap/blob/master/src/logmap.r...](https://github.com/grzegorzk/logmap/blob/master/src/logmap.rs#L90-L93)
could be unwrap_or_else

[https://github.com/grzegorzk/logmap/blob/master/src/logmap.r...](https://github.com/grzegorzk/logmap/blob/master/src/logmap.rs#L101)
should just be `log_filters`, no need for the return;

naming functions with `_` like
[https://github.com/grzegorzk/logmap/blob/master/src/logmap.r...](https://github.com/grzegorzk/logmap/blob/master/src/logmap.rs#L104)
is not idiomatic. It's already not public, no need for `_`.

lines like
[https://github.com/grzegorzk/logmap/blob/master/src/logmap.r...](https://github.com/grzegorzk/logmap/blob/master/src/logmap.rs#L117-L118)
should be one line, no need for the type ascription there.

the package is already called logmap, so
[https://github.com/grzegorzk/logmap/blob/master/src/lib.rs](https://github.com/grzegorzk/logmap/blob/master/src/lib.rs)
is just redundant and adds an extra layer, that is, `logmap::logmap::foo`
rather than just `logmap::foo`.

This isn't an idiom issue, but you might like structopt over getopts.

~~~
gkuk
Many thanks for your great feedback Steve!

I had some issues with running unit tests and at the time I figured if I add
lib.rs I can make my unit tests to run. I did not like logmap::logmap::foo and
definitely need to revisit that.

------
vectorEQ
Not being a rust programmer myself, my comment should be taken as that:

You commented in your code on some long functions needing to be decomposed,
that might be a good start, your own todo. :-)

Other than this as a c programmer i see some opportunity in the
_find_best_matching_filter_index function to perhaps apply a macro to the
repeating syntax of

let words = tst_utils::_words_vector_from_string("aaa bbb ccc ddd");
assert_eq!(log_filters._find_best_matching_filter_index(&words), 0);

Where in C i would create something so i could type MACRO_NAME("aaa bbb ccc
ddd") for example to make that part read more easily and make it easier to add
or remove items there.

I see macros are supported, but am because of my lack of rust knowledge
unaware if my suggestion is possible at all.

[https://doc.rust-lang.org/book/ch19-06-macros.html](https://doc.rust-
lang.org/book/ch19-06-macros.html) might shed some light on that.

It's not really changing code but would make it much more readable. (macros
are great for such things in C for sure if not overdone)

~~~
gkuk
Thanks! I definitely need to be more DRY in tests.

~~~
vectorEQ
I always feel the same. usually only after i wrote something a million times
i'll notice i could have made a macro and end up replacing it all again :D

