The is_not_empty() method as more clearly alternative for !is_empty()

libs
142

Personally I think Rust should do one of these two options:

  • Do nothing.
  • Just add a new function (not a new trait or language syntax). The name of the function doesn't matter much (Rust has made some naming decisions I've disagreed with, but you get used to them and life goes on, so the exact name isn't that important in my opinion as long as it's reasonable).

Adding a trait or language syntax is something that could be explored more, but adding a new (unstable) function to allow experimentation has a much lower barrier to entry (and I personally think we should stop giving Rust new syntax for a year or two) and it can always be deleted if it's deemed not worthwhile.

1 Like
143

I don't think that avoiding language expansion will save this situation; we must unify it at the syntax level, since this point was skipped during the language design.

144

I disagree. There are plenty of languages that don’t have syntax for this sort of thing. Those languages are just fine. Rust doesn’t need syntax for this. Rust’s syntax churn over the past few years has been exhausting. I, for one, need a break.

6 Likes
145

This does remind me of the ExactSizeIterator::is_empty method. That conundrum centers on the fact, that there are more things than just exact size-knowing iterators that know if they are empty or not.

A trait that provides both is_empty and non_empty could solve that problem, by moving is_empty off ExactSizeIterator and into a new trait.

With that said, I don't think a new method should be added. is_empty is good as it is, and reading if !predicate is something that becomes second nature. From the implementer side, we don't gain anything from having more redundant methods in the set of things you have to implement to be a typical collection type. I think we should focus on adding things to Rust that let us do more, not just something that becomes a new style lint in clippy and that's it.

3 Likes
147

Could we implement not() method directly on bool type?

2 Likes
148

It already exists. You can "use" it. There was discussion earlier in the thread about whether or not it should be in the std prelude.

149

bool could have a not() function directly available even without the trait. This would be unnecessary if ops::Not is in the prelude but otherwise I think it would be useful.

150

See https://doc.rust-lang.org/nightly/std/primitive.bool.html#methods for other methods such as bool::then. I think we could do the same to make bool::not work without ops::Not trait.

151

At the beginning of the thread:

In that thread @ExpHP suggests using a pattern that, imho, should indeed be more widespread: redundancy between non-prelude trait methods and inherent methods. bool having an inherent .not() is a good example of this.

3 Likes
152

Could we at least implement is_filled method for arrays and slices?

IMO it might be appropriate method, since arrays and slices cannot be resized, so only two states of them remains possible: empty or completely filled.

153

Arrays and slices cannot be "empty" either. They are always "filled."

154

Is array or slice of length 0 filled?

155

Ah, I see. A mathematical definition of "filled" might be "length equal to capacity," in which case we'd say that a zero-capacity container is "trivially filled." This is the definition used by ArrayVec::is_full, for example.

let a = ArrayVec::<[u8; 0]>::new();
assert!(a.is_full());

This might seem like a curious definition because such a value is both "full" and "empty." However, it is necessary if you want containers to have reasonable properties like "you can insert more items into a container that is not full":

fn add_default<A, T>(a: &mut ArrayVec<A>) where
    A: Array<Item = T>,
    T: Default
{
    if !a.is_full() {
        a.push(T::default()) // can't fail!
    }
}

For fixed-length arrays where the length is part of the type, a method that returns self.len() > 0 seems unnecessary. For slices it might be useful, but for the reasons above I think is_filled would be a confusing name.

156

Also, because a Vec derefs to a slice, any slice method can also be called on a vec. So a slice is_filled method would lead to confusing situations like this:

let mut vec = Vec::with_capacity(256);
vec.push(1);
vec.is_filled() // returns true!
1 Like
157

You implying that "filled" is the same as "full", but I don't think it's true. ArrayVec of length 0 from your example is full, but it's definitely not filled, since for that it should contain something; a mathematical definition of "filled" should be this: "length is greater than 0 and equal to capacity".

What if such array was generated by macro?

Yes, this is valid reason to not implement it, at least until we don't get a straightforward way to opt-out deref on methods like is_filled.

Edit: actually it's possible: since Vec<T> derefs to [T] and not to &[T], items from traits implemented on &[T] aren't directly accessible just by dereferencing Vec<T> (for that we need to use & or as_ref).

So, we could introduce IsFilled (or similar) trait:


trait IsFilled {
    fn is_filled(&self) -> bool;
}

impl <T> IsFilled for  [T; 0] { fn is_filled(&self) -> bool { !self.is_empty() } }
impl <T> IsFilled for  [T; 1] { fn is_filled(&self) -> bool { !self.is_empty() } }
impl <T> IsFilled for  [T; 2] { fn is_filled(&self) -> bool { !self.is_empty() } }
// ...

impl <T> IsFilled for &[T   ] { fn is_filled(&self) -> bool { !self.is_empty() } }

impl     IsFilled for &str    { fn is_filled(&self) -> bool { !self.is_empty() } }


/// Example of IsFilled usage
///
/// ```compile_fail
/// vec![0].is_filled();
/// ```
///
/// ```compile_fail
/// String::from("").is_filled();
/// ```
///
#[test]
fn compiles_fine() {
    [0u8, 1u8]                .is_filled();
    [0u8]                     .is_filled();

    (&[]             as &[u8]).is_filled();
    (vec![].as_ref() as &[u8]).is_filled();
    (&vec![]         as &[u8]).is_filled();

    ""                        .is_filled();
}
158

That is incredibly subtle and seems very bug prone. This introduces an edge case to better fit the natural meaning of filled, but I don't think that it is worth the confusion to do so.

4 Likes
159

is_full exist only in arrayvec crate and this crate even not supposed to include is_filled alongside, so I think you dramatize a bit here about subtlety and error-proneness. Also, I don't see how using natural meaning of "filled" might cause any sort of confusion here, could you elaborate?

160

In general I find that merging two conditions with 1 being an edge case tends to lead to bugs.

In this case, len == capacity and len > 0. In general, you won't run into len > 0, so you won't exercise that condition. But on the off chance you do, it behaves differently. This is bug prone. Better to have a simple rule and stick to it, and call out edge cases explicitly every time.

Finally, there shouldn't be a significant difference between is_filled and is_full, it seems way to easy to miss.

3 Likes
161

But is_filled and is_full wouldn't be "merged" if they would be implemented for different types as I propose: you simply wouldn't be able to call is_filled on ArrayVec and is_full on arrays and slices, so it would be impossible to confuse one with another.

162

ArrayVec derefs to slices just like Vec, so if it is on slices then it is on ArrayVec. I also don't think that this is a good idea for arrays and slices.

In any case, it looks like I misunderstood what you wanted out of is_filled due to this,

But given you edit with an implementation, it looks like is_filled is supposed to mean the same as !self.is_empty(). I don't see enough motivation for this.