riker-rs / riker

Easily build efficient, highly concurrent and resilient applications. An Actor Framework for Rust.
https://riker.rs
MIT License
1.02k stars 69 forks source link

implement is_empty() and use it according to Clippy's prophecy. #98

Open igalic opened 4 years ago

igalic commented 4 years ago

Clippy complains that we have a .len() method, but no corresponding .is_empty() method. Once implemented, it then recommends using !is_empty() in place of len() > 0.

igalic commented 4 years ago

This now leads to:

warning: method is never used: `len`
   --> src/actor/actor_cell.rs:723:5
    |
723 |     pub fn len(&self) -> usize {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

warning: method is never used: `len`
   --> src/actor/actor_cell.rs:723:5
    |
723 |     pub fn len(&self) -> usize {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

so unless we at least create tests and/or rustdoc comments, it can be removed?

leenozara commented 4 years ago

@igalic I think if we remove len()/ we’ll find that we need it back in there again fairly soon. I believe it was used previously on an earlier impl of Context and later lost in a major refactoring.

Context is the “read-only” user view of the actor cell. We could clean up the code as you’ve done, since we don’t want dead code about, or we could look to see how to add the functionality that .len() ultimately could be used for, namely .context.children.len().

Children isn’t part of the Context type, since that is created for the current run of the actor mailbox, but it is part of ActorRef and that is available to Context.

It essentially comes down to updating the API so that user code has access to the number of children the current actor running has, via context.

So, we could remove .len() and then merge this PR in, and come back to exposing number of children in another PR. Or if doing the latter is something you might want to look at then we can hold off on this PR.

What do you think?

igalic commented 4 years ago

Great portions of this comment would make good extensions to the current documentation

I'll to to do as you say, and update the API to be clearer in its intentions.