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

Library improvements #165

Open
bpolaszek opened this issue May 9, 2017 · 8 comments
Open

Library improvements #165

bpolaszek opened this issue May 9, 2017 · 8 comments

Comments

@bpolaszek
Copy link

Hello @danielstjules,

Your library is really great! I'm using it in several projects and it is a must-have swiss knife.

Here's what I suggest for improving it:

  1. On the next major version, rename your package to stringy/stringy. No problem with your username, but I always have to go on github to check the vendor name.

  2. The split() method is very useful, but a join() factory (to do the inverse thing, join an array of Stringy objects) would also be really helpful

  3. Please merge SubStringy methods into the main library. The substring methods are extremely useful (and we expect them when using a string maniuplation library) but the use of a "plugin", which is actually an override requiring to refactor each existing import, is a real pain.

I have made my own fork which integrates these 3 suggestions: https://github.com/bpolaszek/Stringy (I didn't registered it on packagist to let you submit the stringy/stringy package if you agree with my 1st point).
That's what I use in my projects now, and things are way simpler.

What do you think?

@danielstjules
Copy link
Owner

@TCB13
Copy link
Contributor

TCB13 commented May 10, 2017

@bpolaszek I had this SubStringy discussion previously with @danielstjules - the first versions were actually a PR to this repo. There are also other "extensions" that follow this principle.

We're currently in the process of making SubStringy into a trait.

@TCB13
Copy link
Contributor

TCB13 commented May 11, 2017

@bpolaszek @danielstjules TCB13/SubStringy is now a trait.

@bpolaszek
Copy link
Author

Hello @danielstjules @TCB13,

Alright! My 3rd point was about the fact that "substringing" is very common when manipulating strings, that's why I expected these methods to be merged in the main library, allowing the use an out-of-the-box swiss knife, requiring no config and no overriding.

But I understand your point, and the use of a trait is indeed a better approach that a vertical inheritance.

Do you want me to do a PR for points 1 & 2?

Thank you,
Ben

@danielstjules
Copy link
Owner

@TCB13 Awesome! :)

@danielstjules
Copy link
Owner

danielstjules commented May 11, 2017

Alright! My 3rd point was about the fact that "substringing" is very common when manipulating strings, that's why I expected these methods to be merged in the main library, allowing the use an out-of-the-box swiss knife, requiring no config and no overriding.

I definitely think the package is useful, I just think it's hard to come up with terse method names that describe their behaviour. For example:

s('What are your plans today?')->substringAfterFirst('plans ');
// => "today"

// An equivalent using only core methods
s('What are your plans today?')->split('plans ')[1] ?: false

substringCount
// same as countSubstr in Stringy core

// Some of the other methods require a bit more aerobatics with just core,
// so I can see their benefit :)

The core library def has quite a few "substring" methods already :) But I think traits are the right way to go, letting people easily customize at will!

@TCB13
Copy link
Contributor

TCB13 commented May 12, 2017

@danielstjules An equivalent using only core methods those methods are hard on the eyes. This is one of the reasons why I added the substring methods. Easier to use specially when you use an IDE / sublime.

@danielstjules
Copy link
Owner

@TCB13 Agreed! Building off the core methods can get verbose too for some of those operations as the array primitives in PHP aren't great. Definitely appreciate the library :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants