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

What behaviour should occur when all columns are hidden? #19

Closed
tdwright opened this issue Dec 19, 2017 · 5 comments
Closed

What behaviour should occur when all columns are hidden? #19

tdwright opened this issue Dec 19, 2017 · 5 comments
Labels

Comments

@tdwright
Copy link
Owner

Relating to #13 - what behaviour do we want to happen when a data type has public properties (thereby not triggering the exception added in #18), but where the resulting columns have all been manually hidden?

@rahl0b10
Copy link

rahl0b10 commented Dec 19, 2017

Before I commit to a position, I would like to know your reasoning behind having Columns and _colsShown. At first glance I'm inclined to believe it's a YAGNI issue. Unless it currently has a use-case I do not recognize I'd be inclined to simply not add hidden columns to the Columns property in the first place.

public List<Column> Columns { get; set; }

internal List _colsShown => Columns.Where(c => !c.Hide).ToList();

private Table()
        {
            TableStyle = Style.Default;
            var props = typeof(T).GetTypeInfo().DeclaredProperties;
            Columns = props
                .Where(p => p.GetMethod.IsPublic)
                .Select(p => new Column(p.PropertyType, p.Name))
                .Where(c => !c.Hide)
                .ToList();
            
            if (!Columns.Any()) throw new PublicPropertiesNotFoundException();
        }

Since _colsShown isn't part of the API, it wouldn't create a breaking change.

@tdwright
Copy link
Owner Author

Hi @rahl0b10,

The idea behind this design is that we may want to generate a table for certain fields of a pre-existing type. In this case, we'd (indirectly) call the constructor on the type in it's entirety, before marking the Hide property of some columns as true.

For instance, we may want to do as follows:

var listOfData = new List<SomeExistingType>
{
	new SomeExistingType{String1 = "Hello", String2="Baboon", String3 = "World"}
};

var table = ConTabs.Table<SomeExistingType>.Create(listOfData);
table.Columns[1].Hide = true;

Console.WriteLine(table.ToString());

And because we've hidden a column, we get the following:

+---------+---------+
| String1 | String3 |
+---------+---------+
| Hello   | World   |
+---------+---------+

Note that "Baboon" isn't printed.

All the examples and tests contained in the repo use types created ad hoc, solely as vehicles for the data destined for the table. In real usage, these tables are much more likely to receive IEnumerables of types whose main purpose is elsewhere in the codebase. The flexibility to hide certain columns (or format them) is therefore quite important.

Hope this makes sense?

@rahl0b10
Copy link

rahl0b10 commented Dec 19, 2017

Hi @tdwright ,

That does make sense, thank you! You couldn't do what I suggested above because we need the constructed object to set the Hide feature in the first place. In that case, I think I'd fall on the side of throwing a different exception.

@Chandler-Davidson
Copy link
Contributor

I think that it should throw a unique exception that provides a meaningful suggestion.

@tdwright
Copy link
Owner Author

tdwright commented Apr 3, 2018

Fixed in #49

@tdwright tdwright closed this as completed Apr 3, 2018
@ghost ghost removed the backlog label Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants