Refactor `Builder` methods

[zhiburt]

1. Add remove_row remove_column methods.
2. Rename clean method
3. Replace clean with a real clean which would remove all the data
4. (probably) Rename push_record to push_row
5. Add push_column
6. Add insert_column
7. Additionally it would be nice to have a consistent interface with IndexBuilder (One uses &mut self but another self)

[CouldBeFree]

Regarding the first point. It looks like the remove_row and remove_column methods should accept the index.

[zhiburt]

Hi @CouldBeFree, this is correct,

Would you like to try it out?

[CouldBeFree]

Yes, you can assign it to me. I am new to Rust, so If you don't mind I will ask stupid questions during the task.

[CouldBeFree]

@zhiburt how can I test the methods which I added via impl Table?

[zhiburt]

@zhiburt how can I test the methods which I added via impl Table?

There's many ways. The smartest is probably to write actual tests, (including the fact that's relatively straightforward).

See, you could add one more

https://github.com/zhiburt/tabled/blob/74fcf7ade12d3ba5f461f67536f1a32245ef2192/tabled/tests/core/builder_test.rs#L9-L25

[CouldBeFree]

I have a question regarding remove_row and remove_column, as far as those functions update some vec by the given index, should they panic if the index is more than vec length or just do nothing in this case?

[CouldBeFree]

@zhiburt What do you mean by the third point?

Replace clean with a real clean which would remove all the data

As I see this method clear columns and rows. What else is this function supposed to do?

pub fn clean(&mut self) -> &mut Self {
   self.clean_columns();
   self.clean_rows();
   self
}

[zhiburt]

As I see this method clear columns and rows. What else is this function supposed to do?

I did a mistake actually....I was thinking it's called clean.... https://doc.rust-lang.org/std/vec/struct.Vec.html#method.clear

So I think it can stay untouched. We just need to add one more method clear which would remove all the data, but left headers.

PS: If you look what the method does it's not removal, IMHO it's not perfect name but hardly see a better one, maybe you do?

[CouldBeFree]

I have a question regarding remove_row and remove_column, as far as those functions update some vec by the given index, should they panic if the index is more than vec length or just do nothing in this case?

@zhiburt what about this ?

[zhiburt]

I have a question regarding remove_row and remove_column, as far as those functions update some vec by the given index, should they panic if the index is more than vec length or just do nothing in this case?

Good question; Yes.

We just need to add a doc comment about it.

[CouldBeFree]

I have a question regarding remove_row and remove_column, as far as those functions update some vec by the given index, should they panic if the index is more than vec length or just do nothing in this case?

Good question; Yes.

We just need to add a doc comment about it.

I suppose that you mean this -> /// # Panics

[CouldBeFree]

As I see this method clear columns and rows. What else is this function supposed to do?

I did a mistake actually....I was thinking it's called clean.... https://doc.rust-lang.org/std/vec/struct.Vec.html#method.clear

So I think it can stay untouched. We just need to add one more method clear which would remove all the data, but left headers.

PS: If you look what the method does it's not removal, IMHO it's not perfect name but hardly see a better one, maybe you do?

I think about the name like reset or reset_table. So this method should be able to remove all but header?

[CouldBeFree]

@zhiburt I also have question regarding push_column and insert_column methods. As the name suggests push_columns should add a new column at the end, and insert_column should insert a column at the given index. Am i correct?

[zhiburt]

Correct;

Sorry that I have not laid it out more clearly at the outset.

[CouldBeFree]

Correct;

Sorry that I have not laid it out more clearly at the outset.

Don't worry, as you see from my suggestion it's more than clear what should be done. Sorry for disturbing you

[zhiburt]

Hey @CouldBeFree

I wonder if you still have time to do that, or you need some help?

In case you did something already, you could open a PR and I could continue out of that or review it, if you want to.

Take care

[CouldBeFree]

Hi @zhiburt I've implemented 1,2,5,6 points. The clean method I named reset_table. I will create PR, so you will be able to review my code