This is my first project in rust, called: since
. A small tool to filter logfiles or any other files with a timestamp in it, by a given timestamp.
For example:
since 1h /var/log/messages
prints all lines from the last hour.
since 00:00 /var/log/messages
prints all lines since 00:00.
It would be nice if you could take a look at the code. What would you change, what should I change?
I’m not a professional programmer, this is just a hobby, but I want to learn as much as possible.
I took a very quick look at the code and one thing I noticed, is that you’re using
&String
.For a parameter in a function, you always want to use
&str
instead, as explained here: https://doc.rust-lang.org/book/ch04-03-slices.html#string-slices-as-parametersI believe, it’s also not a thing to return
&String
(nor to store it in a struct). I don’t have as solid of an explanation for it, it’s just not something I see much in Rust code (and in the chapter above, they do use a&str
as return value, too).Maybe someone else can weigh in on that.
The same applies for:
&[]
rather than&Vec
&Path
rather than&PathBuf
&OsStr
rather than&OsString
I would also recommend using Clippy. I think, you can just run
cargo clippy
and it’ll work.It’s a linter and will tell you lots of code style issues (like using
&String
as a parameter).If you’ve never run it, it might spit out a lot of warnings at first, but working through them can teach you quite some things.
Thank you very much. I’ll change it. I did run cargo clippy, but it didn’t complained anything anymore before I published the code. 🙂
One question to return value
Option<&String>
:is it better to change to
Option<&str>
or&Option<String>
if the value in the struct is aOption<String>
? The latter sounds more logical to me.@larix @Ephera it’s better to return an Option<&str> as a String may DeRef to &str.
For example
self.name.as_deref()
Hmm, interesting. The documentation tells me, it creates a new Option value, and allocating memory every time someone just wants to look at a value could be pretty bad.
But I guess, an Option of a reference never needs to allocate memory, because it’ll either be a pointer to a value (
Some
) or a pointer to null (None
). Right?Well, that explains why it’s technically possible.
As for why
Option<&str>
is preferrable then:It hides away your internals. Your caller should only care whether they can get the value or not (
Some
vs.None
), not what the precise reason is. That reason or your internal structure might change.@larix
Yes, that makes sense too. Great!
I’ve updated the code as recommended.
@Ephera@lemmy.ml @jcbritobr@mastodon.social
deleted by creator
@Ephera it will be very cheap because you will create only a container with a reference &str to your string. To view data is the preferred way.
Hmm, not quite sure why Clippy didn’t say anything then. I think, it was a
Option<&String>
which I had seen as a parameter type. Maybe it doesn’t complain when it’s wrapped in an Option…Thanks, that was a lot of info I didn’t know either. I was wondering how to use clippy…