Use traits to define common methods?

[2e0byo]

I'm no kind of experienced rust programmer, but I was surprised that common functionality (available, push, try_push) wasn't factored into traits (maybe QueueStats, LimitedQueue and UnlimitedQueue) to allow easily writing generic functions to e.g. get the current queue stats.

(The actual usecase I had was await_empty which I realise will be fixed when #4 lands.)

If this makes sense and you'd merge the PR I'll try to find time to factor the interface out.

[bikeshedder]

Those traits make a lot of sense. Sure, go ahead. I'll gladly merge a PR adding generic traits. 👍

[overhacked]

Throwing up a diagram here for discussion; feel free to quote and edit:

classDiagram

class Queue~T~ {
  <<trait>>
  len() usize
  is_empty() bool
  available() isize
  push(T)
  pop() T
  try_pop() Option~T~
}

note for BoundedQueue "`Result_Unit_T` is a `Result<(), T>`"
class BoundedQueue~T~ {
  <<trait>>
  capacity() usize
  is_full() bool
  try_push(T) Result_Unit_T
}



class `unlimited::Queue`~T~ {
  new() Self
}
`unlimited::Queue` --|> Queue

class `limited::Queue`~T~ {
    new(usize) Self
}
`limited::Queue` --|> BoundedQueue
`limited::Queue` --|> Queue

class `resizable::Queue`~T~ {
    new(usize) Self
    resize(usize)
}
`resizable::Queue` --|> BoundedQueue
`resizable::Queue` --|> Queue

Loading

Do you think that len(), is_empty(), and available() should be extracted into a trait QueueStats? Since all the queue types impl Queue<T>, when would you need the more restricted QueueStats?

[bikeshedder]

Looking at your UML diagram I think only one generic trait is needed.

How about the following behavior for the unlimited queue:

• is_full always returns false
• try_push always succeeds
• capacity always returns usize::MAX

Capacity is the only outlier here. Alternatively capacity() could return an Option<usize> which is None for the unlimited queue.

[overhacked]

I don't like returning Option<usize> because it would be a breaking change, just to add traits, and it's annoying to have to unwrap the capacity on every call.

I like your usize::MAX suggestion, because it's comparison and math operator-friendly. The only footgun I could see...

(ramblings on wrapping addition)

is if someone wrote code for a resizable queue and ran it against an unlimited queue, tried to add a number to `usize::MAX` as the new capacity, and it resulted in wrapping addition under a release binary. You'd get a data loss bug, because you'd unexpectedly truncate the queue. BUT, you'd have to be working with a queue sized close to `usize::MAX`, which seems highly unlikely with this library.

[bikeshedder]

The trait and the impl on the struct don't have to match. As GenericQueue is a new trait it wouldn't even be a breaking change.

It would be somewhat confusing though if there was a queue.capacity() and <queue as GenericQueue>::capacity() that return different types.

So yes... using usize::MAX is probably the smartest choice here.

[overhacked]

Been sketching an implementation of this, and remembering that async functions in traits imply having to make some choices:

1. Adopt a MSRV of 1.75 for the next release and use the new async trait support exclusively.
2. Support a wider range of MSRV by using dtolnay/async-trait exclusively, accepting the performance trade-off of dynamic dispatch that comes with it.
3. Add two new dependencies (one of them dev-only, at least) and use both dtolnay/rustversion to detect whether to use post-1.75 native async in traits support or fall back on async_trait.

[bikeshedder]

I'd either go with 1 or 2. Switching between async-trait and return impl trait is an incompatible change. Detecting the Rust version and switching between those two implementations would result in a breaking change when the user switches between Rust versions.

In deadpool I just made the switch to MSRV 1.75 and removed async-trait as dependency. Though I'm not 100% sold yet as there are severe limitations. Dynamic dispatch is not supported and the resulting trait is not object safe. Thus you can't create a method expecting a &dyn GenericQueue. Though &impl GenericQueue does work as this one doesn't use dynamic dispatch and desugars to <T>(&T).

In deadpool this is a non-issue as Pool already contains the Manager as generic.

With the generic queue interface I do wonder what you need it for. What's the actual use-case for this trait anyways?

[2e0byo]

My original usecase is moot anyhow: I implemented await_not_empty as

async fn await_not_empty<T>(queue: Arc<UnlimitedQueue<T>>) {
    while queue.available() < 0 {
        sleep(Duration::from_millis(100)).await;
    }
}

which is a hack anyway. I have both limited and unlimited queues, so I needed two impls rather than just making the function generic over GenericQueue. The only other use I can think of atm---as a side effect of picking one trait over two---is swapping in an unbounded queue without refactoring, but that's only really something you'd do/I've done in prototyping (to remove a constraint temporarily and see how the system grows).

[bikeshedder]

If you just want to be able to switch between two queue implementations you can create your own module and do a pub use deadqueue::unlimited::Queue; in there. I usually tend to do a type FooQueue = deadqueue::unlimited::Queue<Foo> and use FooQueue in my code.

btw. I'll release a new version of deadqueue tomorrow so you can replace the polling code by using the new functions introduced in #4.