Cell redesign proposal about prettytable-rs HOT 7 OPEN

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

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!

from prettytable-rs.

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

from prettytable-rs.

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/

from prettytable-rs.

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.

from prettytable-rs.

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.

from prettytable-rs.

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 ?

from prettytable-rs.

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).

from prettytable-rs.