Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should the inner state be public? #29

Open
btrepp opened this issue Jan 11, 2022 · 1 comment
Open

Should the inner state be public? #29

btrepp opened this issue Jan 11, 2022 · 1 comment

Comments

@btrepp
Copy link

btrepp commented Jan 11, 2022

First off Amazing library!. I stumbled upon it when writing my own proc macros to try a similar goal, so very happy a much better solution already existed.

One thing I've noticed that access to the nested states and values typically does seem to be accessible.

This comes from


Most examples also have the inner 'states' having public variables, so these end up being accessible too.

In btrepp@eb92c15 I was able to remove pub, and fix examples to work. The biggest trick is making the implementations of the trait to be inside the mod itself, thus allowing access to any struct field without declaring them pub. This seems to keep everything nice and hidden, meaning we can only use our public apis to interact with the typestate machines. The downside is trait impls must be inside the mod block, which isn't as nice. There may be ways of working around that using a function macro instead of an attribute one.

I didn't open this as a PR yet, as perhaps there are valid reasons to have the inner state accessible, but at a glance I feel like the pattern should be forcing API consumers to follow the state transitions, and hiding it's inner properties.

Cargo expand on main lightbulb example

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
use light_bulb::*;
use typestate::typestate;
///```mermaid
///stateDiagram-v2
///[*] --> Off : screw
///Off --> On : turn_on
///Off --> [*] : unscrew
///On --> Off : turn_off
///```
mod light_bulb {
    pub struct LightBulb<State: LightBulbState> {
        pub state: State,
    }
    pub struct Off;
    pub trait OffState {
        fn screw() -> LightBulb<Off>;
        fn unscrew(self);
        #[must_use]
        fn turn_on(self) -> LightBulb<On>;
    }
    pub struct On;
    pub trait OnState {
        #[must_use]
        fn turn_off(self) -> LightBulb<Off>;
    }
    #[doc(hidden)]
    mod __private {
        pub trait LightBulbState {}
    }
    pub trait LightBulbState: __private::LightBulbState {}
    impl<__T: ?::core::marker::Sized> LightBulbState for __T where __T: __private::LightBulbState {}
    impl __private::LightBulbState for Off {}
    impl __private::LightBulbState for On {}
}
impl OffState for LightBulb<Off> {
    fn screw() -> LightBulb<Off> {
        Self { state: Off }
    }
    fn unscrew(self) {}
    fn turn_on(self) -> LightBulb<On> {
        LightBulb::<On> { state: On }
    }
}
impl OnState for LightBulb<On> {
    fn turn_off(self) -> LightBulb<Off> {
        LightBulb::<Off> { state: Off }
    }
}
fn main() {
    let bulb = LightBulb::<Off>::screw();
    let bulb = bulb.turn_on();
    let bulb = bulb.turn_off();
}

Cargo expand on my fork that changes pub state: State to state:State

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2018::*;
#[macro_use]
extern crate std;
use light_bulb::*;
use typestate::typestate;
///```mermaid
///stateDiagram-v2
///[*] --> Off : screw
///Off --> [*] : unscrew
///Off --> On : turn_on
///On --> Off : turn_off
///```
mod light_bulb {
    pub struct LightBulb<State: LightBulbState> {
        state: State,
    }
    pub struct Off;
    pub trait OffState {
        fn screw() -> LightBulb<Off>;
        fn unscrew(self);
        #[must_use]
        fn turn_on(self) -> LightBulb<On>;
    }
    pub struct On;
    pub trait OnState {
        #[must_use]
        fn turn_off(self) -> LightBulb<Off>;
    }
    impl OffState for LightBulb<Off> {
        fn screw() -> LightBulb<Off> {
            Self { state: Off }
        }
        fn unscrew(self) {}
        fn turn_on(self) -> LightBulb<On> {
            LightBulb::<On> { state: On }
        }
    }
    impl OnState for LightBulb<On> {
        fn turn_off(self) -> LightBulb<Off> {
            LightBulb::<Off> { state: Off }
        }
    }
    #[doc(hidden)]
    mod __private {
        pub trait LightBulbState {}
    }
    pub trait LightBulbState: __private::LightBulbState {}
    impl<__T: ?::core::marker::Sized> LightBulbState for __T where __T: __private::LightBulbState {}
    impl __private::LightBulbState for Off {}
    impl __private::LightBulbState for On {}
}
fn main() {
    let bulb = LightBulb::<Off>::screw();
    let bulb = bulb.turn_on();
    let bulb = bulb.turn_off();
}
@jmg-duarte
Copy link
Contributor

Hello @btrepp! Glad to know that you found my crate interesting (I've spent a long time on it).

To provide you with a brief answer, the inner state can carry data thus I made it pub by default.

In a more complete reply - I agree that the inner data could be private, ideally, the user should have full control over it.

Addressing your impl idea. I have nothing against it. In fact, I wanted to do that from the begining. However, it raises some questions:

  • What happens with functions? Should they return State or Automaton<State>?
  • And "meat" of the functions? How can they return the correct type with minimal impact for the user?

To materialize what I was talking about:

trait S {
    fn new() -> S;
}

impl S for Automaton<S> {
    // The signatures don't match pre-expansion and this might be confusing
    // One can also expand them so we could simply have 
    // ```
    // impl S { ... }
    // ```
    // But then the question is, what about the inside function?
    // How can the user *easily* return a valid `Automaton<S>`?
    fn new() -> Automaton<S> { 
        // ...
    }
}

Let me know your thoughts on the questions above!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants