zhiburt / tabled

An easy to use library for pretty print tables of Rust structs and enums.
MIT License
1.94k stars 77 forks source link

Table::count_rows() includes the header row #421

Open jonner opened 1 month ago

jonner commented 1 month ago

It feels a bit surprising that table.count_rows() would not return only the number of data rows, but also include the header row. For example:

fn main() {
    let table = Table::new(["foo"]);
    println!("{} row(s)", table.count_rows());
}

Result: 2 row(s)

Expected: 1 row(s)

jonner commented 1 month ago

Also, surprising that the Table::new() and Builder have slightly different behaviors with respect to headers and row counts

Table::new

    let mut table = Table::new([["foo", "bar"]]);
    println!("{table}");
    println!("{} records found", table.count_rows());

Output:

+-----+-----+
| 0   | 1   |
+-----+-----+
| foo | bar |
+-----+-----+
2 records found

Builder

    let mut builder = Builder::new();
    builder.push_record(["foo", "bar"]);
    let table = builder.build();
    println!("{table}");
    println!("{} records found", table.count_rows());

Output:

+-----+-----+
| foo | bar |
+-----+-----+
1 records found
zhiburt commented 1 month ago

Hi @jonner

I'd certainly agree that there might be some misunderstandings

Result: 2 row(s)

Expected: 1 row(s)

I see why you'd expect 1 row, As I remember, there was a time where it was a case (ether for Builder or for Table)

But I think that's correct for both Table and Builder to return a real number of rows will be rendered, including column names. Don't you think?

Like I don't see a case where you'd want a different result. Maybe beside the one where you consume iterator Table::from_iter and wanna know it, but if you call it you always know that there will be header anyhow so it's +1

Maybe we shall emphasize it better in the docs.

What do you think?

zhiburt commented 1 month ago

We could add a new method smth. as count_records which would not include the header. But I kind of doubt it (quite a bit).

Anyhow waiting on your thoughts.

Take care

jonner commented 1 month ago

Like I don't see a case where you'd want a different result.

In my case, I was creating a table and adding data to it, and then I wanted to print a summary at the bottom indicating how many records were in the table. And I didn't notice until later that it was always off by one due to the header row. I fixed this by just counting the data I added to the table rather than using the Table::count_rows() API. No big deal, but it was surprising to me.

I personally can't think of a case where you'd actually want the value to include the header row as a user of the API. The only thing I could think of was if you were trying to calculate how many rows it would take to render the table, but that isn't accurate because the value doesn't include the separator rows of your chosen style, etc.

I feel like the automatic header row makes the API a bit awkward and unexpected. The table data and the table metadata (column headers) are fundamentally different things to me. And the fact that they're treated as one big chunk of data (with the header row sometimes automatically inserted and sometimes not, depending on which API you use) makes things behave in subtly unexpected ways. But I can work around the count_rows() behavior that I originally reported. The difference in behavior between the Table API and the Builder API seems more problematic:

println!("with Builder::from_iter()");
let builder = tabled::builder::Builder::from_iter([
    ["Item 1", "value 1"],
    ["Item 2", "value 2"],
    ["Item 3", "value 3"],
]);
println!(
    "{}\n",
    builder.build().with(tabled::settings::Style::psql())
);

println!("with Table::builder()");
let builder = tabled::Table::builder([
    ["Item 1", "value 1"],
    ["Item 2", "value 2"],
    ["Item 3", "value 3"],
]);
println!(
    "{}\n",
    builder.build().with(tabled::settings::Style::psql())
);

Output:

with Builder::from_iter()
 Item 1 | value 1 
--------+---------
 Item 2 | value 2 
 Item 3 | value 3 

with Table::builder()
 0      | 1       
--------+---------
 Item 1 | value 1 
 Item 2 | value 2 
 Item 3 | value 3 
jonner commented 1 month ago

I know this is a tangential issue from the original report, so I can open a new bug, or you can just close the bug if you want. But it all relates to the fact that the data and metadata are intermingled in the internal data store.