sorensen / ascii-table

Ascii Tables for JS
MIT License
185 stars 31 forks source link

Remove alignLeft, alignCenter, alignRight, alignAuto #14

Open gajus opened 9 years ago

gajus commented 9 years ago

Remove:

alignLeft(val, len, [pad])
alignCenter(val, len, [pad])
alignRight(val, len, [pad])
alignAuto(val, len, [pad])
setAlignLeft(idx)
setAlignCenter(idx)
setAlignRight(idx)
setTitleAlignLeft()
setTitleAlignCenter()
setTitleAlignRight()
setHeadingAlignLeft()
setHeadingAlignCenter()
setHeadingAlignRight()

None of the methods provide value to the package. It only makes the API looks less approachable and use less predictable. Removing these methods in favour of keeping just align, setAlign, and setHeadingAlign would reduce the complexity of the API.

sorensen commented 8 years ago

Agreed, the new refactor will eliminate all of these methods, including align, setAlign, and setHeadingAlign. There will be a single table configuration method to set all of these values.

gajus commented 8 years ago

Any reason to refactor ascii-table than to continue work on https://github.com/gajus/table, which already has modular code base?

sorensen commented 8 years ago

My primary reason is that I intend this codebase to be a single file exportable to both node and the browser, and would like to make this codebase as minimal as possible. I like what you are doing with the table package, but I think you and I differ on how a project should be structured and the API it should have.

One of my primary issues with the table package is how modular it is, this should be a very small codebase and not require multi-file abstraction, in my opinion.

This module was build many years ago, and I intend to upgrade it with what I know now along with your suggestions on API design.

sorensen commented 8 years ago

@gajus Would you mind having a more in depth discussion about this? I'm not sure github comments are a good discussion or brainstorming tool, send me an email at mail@beausorensen.com I think we can at the very least improve both our libraries

gajus commented 8 years ago

My primary reason is that I intend this codebase to be a single file exportable to both node and the browser.

This sounds like a case of over-engineering. If the intention is to have a shared API between server-side code and client-side code, then have two separate packages that implement the same interface.

I think you and I differ on how a project should be structured and the API it should have.

If your concern is the API, then raise an issue and propose a different API/ additions to the existing API. It must mention the benefits of the proposed changes over the existing API. Furthermore, the underlying implementation is granular enough to enable pretty much whatever API. Proposing changes to an existing project is a lot easier first step to take then to write/ reimplement yet another library.

One of my primary issues with the table package is how modular it is, this should be a very small codebase and not require multi-file abstraction, in my opinion.

All modules are implemented following principles of functional programming (namely: single-responsibility, no side effects, no state).

@gajus Would you mind having a more in depth discussion about this? I'm not sure github comments are a good discussion or brainstorming tool, send me an email at mail@beausorensen.com I think we can at the very least improve both our libraries

Thank you for the invitation to the discussion. However, given that we are discussing open-source projects, I think that GitHub comments is an appropriate place to have such discussion.