Philosophical question about the Default trait - and its clippy warning

I got the following clippy warning and previously fixed it with #[derive(Default)] as suggested.

warning: you should consider deriving a `Default` implementation for `Ulid`
   --> src/lib.rs:243:5
    |
243 | /     pub fn new() -> Ulid {
244 | |         Ulid::from_timestamp_with_rng(unix_epoch_ms(), &mut rand::thread_rng())
245 | |     }
    | |_____^
    |
    = note: #[warn(clippy::new_without_default_derive)] on by default
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#new_without_default_derive
help: try this
    |
212 | #[derive(Default)]
    |

This seems counter-intuitive since new() returns different values containing timestamp and random bits on every call whereas I’d somewhat expect Default::default() to return the same value on every call, like 0 being the default for i32.

A (0, 0) default Ulid doesn’t really make sense, though, because it would be rather useless…

A useful manual Default implementation would instead look like this:

impl Default for Ulid {
    fn default() -> Ulid {
        Ulid::new()
    }
}

The Default trait itself doesn’t explicitly prohibit returning different values but it feels semantically wrong, while providing a new() seems fine to me.

For now, I disabled the clippy warning instead of implementing the Default trait either derived or manually.

What do you think?
Is this an exception to the generally reasonable new_without_default_derive clippy rule or am I overthinking the semantics of Default?

3 Likes

I’d narrow it down to two possibilities:

  • The lint is too strict/this is an exception.
  • Or you should rename new. (e.g. generate)

Deriving Default is out of the question, and making it a shim for new in this case sounds plain wrong.

8 Likes

The clippy warning is acting more like a reminder in this case. You’ve considered deriving Default, and it sounds reasonable that you shouldn’t have it, so I would just silence the warning here. (Which would also serve to document your intention that Default is intentionally not derived.)

6 Likes

I concur: Default should always retrun the same value, but new not necessarily. I’d just silence it as well.

It might be useful as a placeholder, however, similar to how Uuid::EMPTY is often used.

That said, if clippy is expecting it, humans might also be expecting it, so I like @ExpHP’s suggestion to consider a different name.

2 Likes

Yeah, when I have a constructor with side-effects, I usually call it something like “create” or “gen”, just so it’s distinct from “new”.

1 Like

And eventually be (when const functions will be more powerful) const?

Unfortunately, that would be a breaking change.

2 Likes

That’s a really good point. I renamed new() to generate() and released a new version.

Thanks for the feedback, folks!

1 Like

Did you say philosophical... :wink:

I agree with the advice but...

We are in the territory of programming standards and best practices.

It literally depends on what level you ask. For a language and low level its debatable but a convention.

For a complex object type like a a customer class this would be counter intuitive as I am sure you know.

In one (c#) prototype every object had a (default) new but a flag to make sure "init" was called (our generate equivalent.)

This was consistent in a different way. You could trust new().

I checked the documentation again. It seems to me a very powerful feature.

This was meant for frameworks and complex objects as well as primitive and simple types.

I don't object to the (API?) convention but I would not make any assumptions.

Slightly off-topic: I find the notion of "placeholder"/"empty"/sentinel/otherwise invalid and/or nonsensical values dangerous and an anti-pattern in Rust. I'm not fond of either floating-point NaN, null pointers, or the null UUID provided by the Uuid crate for the same reason — they are analogous bads.

Rust has an explicit and strongly-typed alternative to signal a missing/nonexistent/invalid value: Option<T>. Using an optional IMO is superior because it forces you to consider/handle the invalid value, hence it's free of surprises.

ON: I also think disabling the lint is the correct solution in this case. Sometimes, disabling is the right choice.

3 Likes

Lol. I found Rust and I went "Look! A language designed to handle nulls!!! And results!"

... Billions of wasted industry dollars later. Chuckle.

So I agree as a noob without doubt. I will watch that thanks.

Please allow me to consolidate what I found here.

This link helped. It explains it (Default) is needed for generics whereas you can normally call new(?) on your regular objects and don't need it.

Excuse the link. On mobile I have compose in my clipboard and then paste. Groan.

Anyway, per the Rust docs, Clippy should only issue that warning for generics unless you want to enforce explicit declarations of some sort.

Is this correct?