Cell redesign proposal

[hcpl]

This is a proposal to remake Cell and related functionality to simplify fixing issues #45, #46 and possibly #47 (since this proposal, if accepted, will allow defining custom behaviour of cell elements in case of the outer table being shrinked).

Here are some ways how this proposal can be implemented.

1. Statically dispatched trait-based

trait CellContent { ... }
impl CellContent for Vec<String> { ... }    // represents lines
impl CellContent for Table { ... }
...

struct Cell<T: CellContent> {
    content: T,
    ...
}

• Pros:

  • Easy for implementation: you don't have to care about the possibility of having values of different types in any given moment of time.
  • Safe: the table will behave exactly like you (author) designed it to be, since everything down to cells is verified by you.
  • Semi-easy to adapt existing public API:
  • Serious changes are likely to be required only for Cells, for other structs they're either avoidable, or have to do something with formatting and styling.
  • Thanks to this StackOverflow answer external appearance of cell! macros can remain the same.

• Cons:

  • Unusable if you need differently typed contents, since the cell itself is locked to one type and because of the reason below. (Well, unusable if you don't want to convert everything to String. If this is the case this mode works perfectly fine)
  • Semi-difficult to adapt existing public API:
  • Since Table's and Row's backing storage is a homogeneous Vec, the <T> parameter would be needed to spread across the whole API.

2. Dynamically dispatched trait-based

For trait definition see above

struct Cell {
content: Box<CellContent>,    // could be other box structure here
...
}

• Pros:

• Flexible: users will be able to define new types with their own behaviours in the table.
• Easy to adapt existing public API:
• Cell constructors: types can be backwards-compatibly generalized, whereas the amount of constructors remains the same.
• This means cell! macro wouldn't have to be modified at all.
• And (probably) everything else wouldn't change neither.

As a result, public API changes would be minimal and backwards-compatible.

• Cons:

  • Ineffective: Cell has to store CellContent boxed since we wouldn't know the exact type. This would certainly have performance implications, though their extent varies among usages.
  • Mildly unsafe: there should be some precautionary measures against bad CellContent implementations.

3. Enum-based

enum CellContentBox {
    Text(Vec<String>),    // represents lines
    Table(Table),
    ...
}

impl CellContentBox { ... }

struct Cell {
    content: CellContentBox,
    ...
}

• Pros:

  • Effective: theoretically, everything in this case can be stored on stack.
  • Safe: same note for Statically dispatched trait-based.

• Cons:

  • Inflexible: there is no easy way to add new types. You have to manually add a variant to CellContentBox and add implementation into all related functions and methods, which is a more cumbersome process than implementing a trait. It also makes code less clean and readable.
  • Difficult to adapt existing public API:
  • Cell constructors would probably have constructors for every enum variant.
  • Macros aren't capable of detecting types of input values, so this forces having a dedicated cell_*! for every enum variant.

UPD: More info about macros and wording.

[phsym]

Hi ! I've read this very quickly on my phone. I need to read again more carefully, but it seems very interresting and legit. I'm not sure which of the 3 propositions I prefer. As you said, they all have their pros and cons. Offerring the maximum flexibility to library users would probably be the best.

By the way, regarding the cons around the Enum-based solution, I think we could implement the From conversion trait on the enum and use it in the cell! macro instead of implementing a macro for each variant. Also, regarding the inflexibility, we could still have a variant for generic trait objects. Like some kind of mix with your 3 proposals. Just throwing ideas. I'll study your proposal more carefully later today. Thanks

[hcpl]

From really looks good to me (I've forgotten the obvious example of try!).

Did I get it right that by "a variant for generic trait objects" you meant something like CellContentBox::Other(Box<CellContent>)? If it really is... Well, it doesn't look elegant, but seems like a viable compromise.

Ah, and macros in all 3 cases can work without modifying external API. \o/

[phsym]

Yes,that's what I meant, but I'm agree it's not really elegant.

I've been thinking on this on the way back to home, and maybe we can mix your 3 proposals into 1, and let the library user choose. Let me explain:

Regarding your first proposal, the plan is to define something like

struct Table<T: CellContent> { ... }

right ? So by implementing the CellContent over appropriate types we can have

Table<Vec<String>>
Table<Table<Vec<String>>
// An so on ...

As you said, we would have to spread the type accross the entire API, but I'm not fully agree when you say we would be locked to one single type. In fact we should be able to write something like

impl <T> CellContent for Box<T> where T: CellContent + ?Sized { ... }
// Then use
Table<Box<CellContent>>

This would enable statically AND dynamically dispatched trait-based solution. Merging together your proposals 1 an 2. The library user would decide which kind of usage is more approriate to the situation.

We can also hide it behind a type like

pub type BoxTable = Table<Box<CellContent>>;

And finally, your third proposal can also join the party since the enum can also implement CellContent. With it, the lib user would be able to opt for a "multi-typed content" but statically dispatched solution.

[hcpl]

Wow, combining the good parts of different solutions to get the best one. I feel proud for being treated like that, thank you! :smile:

Now that I think of it, most of the pros and cons I've shown stem from the question "How to deliver the desired feature with breaking API as less as possible, ideally not breaking at all, at the same time trying not to sacrifice raw performance?" So far I only considered dynamic dispatch + enum, because it can be done implicitly albeit giving up on effectiveness. But if genericity is worth going all out, then I'm on it!

[hcpl]

Current implementation progress:

• Static dispatch (only)

  Converting Table to Table<T> and using Vec<String> as a type parameter works as it worked before, although with a small cheat: I had to create

  struct CellLines {
    lines: Vec<String>,
  }

  to implement

  impl<T: ToString> From<T> for CellLines {
    fn from(value: T) -> CellLines {
        CellLines { lines: value.to_string().lines().map(str::to_owned).collect() }
    }
  }

  for convenience, so the actual table is Table<CellLines>. Also this means inner tables are converted to strings, so no custom behaviour available.

• Dynamic dispatch (only)

  Given

  impl<T: CellContent> From<T> for Box<CellContent> {
    fn from(content: T) -> Box<CellContent> {
        Box::new(content)
    }
  }

  I wanted to do:

  impl Cell {
    fn new<T, U>(value: U) -> Cell
        where T: From<U> + CellContent
    {
        Cell { content: Box::new(T::from(value)) }
    }
  }

  but this definition is ambiguous in T, so I had to make a more strict one:

  impl Cell {
    fn new<T: CellContent>(content: T) -> Cell {
        Cell { content: Box::new(content) }
    }
  }

  With this we have to use, for example, Cell::new(CellLines::from("many\nlines")) instead of Cell::new("many\nlines") or Cell::new(From::from("many\nlines")). Cell::new(table) still works though.

  As another solution, I tried

  impl<T: ToString> From<T> for Box<CellContent> {
    fn from(value: T) -> Box<CellContent> {
        Box::new(CellLines::from(value))    // uses From<T> for CellLines above
    }
  }

  However, this definition along with impl From<Table> for Box<CellContent> results in conflicting implementations because Table implements both CellContent and ToString.

  This is a blocker issue for me, since I can't think of any other way other than impl specialization, which is not even done yet. As a result, it will to be difficult to make a catch-all cell! implementation. I hope there can be other working solutions which retain a user-friendly API.

• Enum

  Practically the same issue as with dynamic dispatch, only with enum CellContentBox presented in the proposal.

[phsym]

Hi ! Sorry for the late reply, I've been quite busy this week. I havent had time yet to start experimenting around this redesign. I'll give it a look ASAP. In the meantime could you share your code ?

[hcpl]

The code is here: https://github.com/hcpl/prettytable-rs/tree/cell-redesign. One notable API change to consider there is allowing table! and cell! to accept a type hint argument (though couldn't figure out how to do this for row! because of its vector-like syntax).