Improvements on Orders methods

[MuhannadAlrusayni]

Hi, since we are about to release 0.7 I would suggest renaming some Orders methods, for clarity and readability and consistently, here is some better names:

• send <-> send_msg
• async_send <-> perform_cmd

and I would suggest to add 2 more methods:

• send_after(ms, handler) <-> async_send(cmds::timeout(ms, handler))
• stream_every(ms, handler) <-> stream(streams::interval(ms, handler))

and not to forget *_with_handle() methods.

[MartinKavik]

Counter-arguments:

• send <-> send_msg
  • send may be confusing - Send msg? Send request? Send notification?
• async_send <-> perform_cmd
  • You don't have to send anything - handler can return Msg, Option<Msg> or nothing.
• send_after, send_after_with_handler, stream_every, stream_every_with_handler
  • Is writing perform_cmd(cmds::timeout( and stream(streams::interval( so annoying that we want to introduce 4 new public redundant methods?
  • It's not possible to e.g. map stream / command in send_after* and stream_every*.

[MuhannadAlrusayni]

• send <-> send_msg I think send consistent with stream and notify naming in an expected way. actix framework also use send to send message, users used to the actor system would expect this to send message.

  send may be confusing

  I don't think it's confusing since Orders talk with the runtime using message, so I would expect that send take message.

  Send request?

  I don't think someone would expect Orders it to send HTTP request(if that's what you mean?).

  Send notification?

  same goes for notification which have notify() and not notify_msg().

• async_send <-> perform_cmd

  You don't have to send anything - handler can return Msg, Option<Msg> or nothing.

  Here is an alternative names cmd or async_cmd if that desired, the point is to make it clear that this method do an async block and try to make it shorter since it's commonly used method.

  Note that this argument also apply to stream method since we can stream nothing (e.g. (), None).

• send_after, stream_every

  Is writing perform_cmd(cmds::timeout( and stream(streams::interval( so annoying that we want to introduce 4 new public redundant methods?

  Nope, but it's more convenient to write and read send_after(..) than perform_cmd(cmds::timeout(..)) and this is common thing to do. Although I don't like stream_every_with_handle name.

  It's not possible to e.g. map stream / command in send_after* and stream_every*.

  I'm not sure what you trying to say here. would you elaborate on this.

[MartinKavik]

I'm not sure what you trying to say here. would you elaborate on this.

You can modify cmd / stream after the creation - .stream(streams::interval(1000, || Msg::OnTick).do_something()); - but yeah, it's an edge case.

---

Nope, but it's more convenient to write and read send_after(..) than perform_cmd(cmds::timeout(..)) and this is common thing to do.

You can say the same about sending HTTP requests, subscribing to UrlChanged, etc. => I would rather see orders.send_request( or order.on_url_changed( as convenient functions because I use them much more often. But I'm afraid that with each added method it will be harder to understand / read the code.

---

Other opinions? (@TatriX, @rebo, @Ben-PH, @power-fungus, and others)

[samuelpilz]

Personally, I like send_msg and perform_cmd. I agree that they are not too consistent with stream. Instead, I suggest changing that to subscribe instead. This is an argument of taste. My taste is more close to elm.