search

muesli4 / table-layout

Layout data in grids and pretty tables. Provides a lot of tools to get the cell formatting right (positional alignment, alignment on specific characters and limiting of cell width)..
BSD 3-Clause "New" or "Revised" License
38 stars 11 forks source link

Add explicit defaults for various data types #26

Closed Xitian9 closed 1 year ago

Xitian9 commented 3 years ago

instead of exclusively relying on the Default typeclass

This allows anybody who wants to use the default values without relying on typeclasses to do so. These more explicit defaults are then used in code.

Xitian9 commented 2 years ago

This is low priority, so I've marked it as draft to avoid making conflicts for #27.

Xitian9 commented 2 years ago

Now that #27 has been merged, this is ready for review.

muesli4 commented 2 years ago

Could you explain why someone would want to use this? Is it for improved compiler error messages? Or perhaps because they do not want the dependency? Then we would require another package that contains all the default definitions. (I would have to think about how this would impact visibility.) Personally, I am not a huge fan of those names, although that is probably meaningless. But if the only reason to do this is to get table-layout on stackage then I am not sure whether this should be done.

Xitian9 commented 2 years ago

The reason is to help with code comprehension and type inference. def is not a very informative name, and whenever there's a value which is known only by the name def, I need to go into the code to find the definition. Before this PR, this is the case with the defaults of type HeaderSpec, ColSpec, and CutMark. It also introduces the possibility for errors. If I want a table with default HeaderSpec but not default ColSpec, or vice versa, I would create one of them with repeat def. It is very easy to mess up the order of the constructions and put the wrong one in there.

Current trends are to reduce reliance on the Default typeclass, and I take this PR as implementing that. I don't see any need to remove typeclass instances, I just thought that every default value should have a proper name that wasn't def, and that these more explicit names should be used within the code (so long as they're there).

That said this is not a huge issue, and if you feel that you would rather not have this PR I'm happy to abandon it.

Xitian9 commented 2 years ago

Also, I'm happy to change any of the names.

simonmichael commented 1 year ago

I quite like the convenience and terseness of def myself, but it's true that many haskellers avoid it, eg because of the impact on comprehensibility and error messages. Many packages prefer providing explicit monotyped bindings instead. That said, this particular change might not be urgent if table-layout's def is not causing problems for anyone.