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

Methods to work with SubStrings, find Offsets and Evaluate String Contents #87

Closed
wants to merge 7 commits into from

Conversation

TCB13
Copy link
Contributor

@TCB13 TCB13 commented Apr 9, 2015

Added multiple methods to work with substrings, find offsets and evaluate string contents. fixes #86

/**
* Returns the offset/index of the last occurance of $substr in the string.
* In case $substr is not a substring of the string, returns false.
*
* @param string $substr substring
* @param int $offset
* @return int|bool
*/
public function offsetOfSubstr($substr, $offset = 0);

/**
 * Returns the offset/index of the last occurance of $substr in the string.
 * In case $substr is not a substring of the string, returns false.
 *
 * @param string $substr substring
 * @param int $offset
 * @return int|bool
 */
public function offsetOfLastSubstr($substr, $offset = 0);

/**
* Returns true if the string is empty (doesn't contain any char),
* false otherwise.
*
* @return bool Whether or not $str is empty (no chars)
*/
public function isEmpty();

/**
 * Returns true if the string contains an integer value, false
 * otherwise.
 *
 * @return bool Whether or not $str contains an integer value
 */
public static function containsInt();

/**
 * Returns true if the string contains an float value, false
 * otherwise.
 *
 * @return bool Whether or not $str contains an float value
 */
public static function constainsFloat();

 /**
 * Gets the substring after the first occurrence of a separator.
 * If no match is found returns false.
 * 
 * @param string $separator
 * @return string|bool
 */
public function substringAfterFirst($separator);

/**
 * Gets the substring after the last occurrence of a separator.
 * If no match is found returns false.
 * 
 * @param string $separator
 * @return string|bool
 */
public function substringAfterLast($separator);

/**
 * Gets the substring before the first occurrence of a separator.
 * If no match is found returns false.
 * 
 * @param string $separator
 * @return string|bool
 */
public function substringBeforeFirst($separator);

/**
 * Gets the substring before the last occurrence of a separator.
 * If no match is found returns false.
 * 
 * @param string $separator
 * @return string|bool
 */
public function substringBeforeLast($separator);

Tadeu Bento added 4 commits April 9, 2015 23:15
…uate string contents. fixes danielstjules#86

offsetOfSubstr();
offsetOfLastSubstr();
isEmpty();
containsInt();
constainsFloat();
substringAfterFirst();
substringAfterLast();
substringBeforeFirst();
substringBeforeLast();
@danielstjules
Copy link
Owner

I appreciate you taking the time for PHP 5.3 support. I realize it's been EOL'ed, but I'd rather not release a new major to drop support. I'll take a look at the PR :)

@TCB13
Copy link
Contributor Author

TCB13 commented Apr 16, 2015

It was about a 2-line fix after knowing what was going on, so not a big deal there. I was more concerned about the performance impact of this fix than anything else...

Waiting your feedback!

@TCB13
Copy link
Contributor Author

TCB13 commented May 1, 2015

Any news on this request?

@danielstjules
Copy link
Owner

Sure, sorry again!

I'm not sure I see the benefit of isEmpty, containsInt, and constainsFloat (should be containsFloat). Do you find yourself needing a multibyte string library for that kind of stuff? Also, is containsFloat a bit complicated - can't you use floatval? Or are does it not work in the case of some encodings, or mismatch?

$ php -a
Interactive shell
php > echo floatval('1.337fòô bàř') . "\n";
1.337

I like the idea of strpos/strrpos methods (which you named offsetOfSubstr/offsetOfLastSubstr), but I might change their name to be more concise, or even combine them. I'm wondering if they could maybe become index, or indexOf, and just use a third arg to determine whether or not it returns the first or last occurrence.

As for the substring(Before/After)(First/Last) methods, I think I'd rather leave them out, and maybe recommend their usage/inclusion as part of a subclass. If you wanted to include them in a separate lib, I'd be happy to link to it in https://github.com/danielstjules/Stringy#links :)

@TCB13
Copy link
Contributor Author

TCB13 commented May 4, 2015

Hi!

  • isEmpty, containsInt, and constainsFloat (yeah small typo...) => It's way easier to use this methods instead of making the comparisons etc every single time...
  • "needing a multibyte string library for that kind of stuff" - well, alone no. But we always have to deal with string... and since in a lot of use cases floats and ints come inside strings (user input, mysql...), and we already have the "multibyte string library" loaded why not also use it for this...?
  • "can't you use floatval" - It will behave like a (float) cast... and it doesn't handle floats with thousand separators.
  • offsetOfSubstr and offsetOfLastSubstr to index, or indexOf => I don't see why not :)
    The merge is also possible, however it will be... annoying. The methods implement a $offset = 0 default parameter, if we merge them we'll have something like offsetOfSubstr($substr, $offset = 0, $last = false) or offsetOfSubstr($substr, $last = false, $offset = 0) that will always force you to manually define sometime that usually is automatically done...
  • About substring* methods: IMHO those methods are the next step after the offsetOfSubstr. Actually the only reason why I implemented offsetOfSubstr methods were to support this substring methods... They are usually very useful when parsing complex text input.

Waiting your feedback!

@TCB13
Copy link
Contributor Author

TCB13 commented May 10, 2015

@danielstjules I really would like some feedback on the last message to decide what to do. Thanks.

@danielstjules
Copy link
Owner

isEmpty, containsInt, and constainsFloat (yeah small typo...) => It's way easier to use this methods instead of making the comparisons etc every single time...

I disagree. You still have to do the comparison every time, since they're not chainable.

$a = S::create('');
$b = S::create('');

if ($a->isEmpty() && $b->isEmpty) throw new Exception('...');

if ($a === '' && $b === '') throw new Exception('...');

Not only is the comparison shorter, but the behavior is a lot more clear, as I'm not having to look up the implementation or signature for some method.

"can't you use floatval" - It will behave like a (float) cast... and it doesn't handle floats with thousand separators.

Your example test cases didn't show an example with multiple separators. Beyond that, I don't think it's a necessary divergence from the standard lib.

offsetOfSubstr and offsetOfLastSubstr to index, or indexOf => I don't see why not :)
The merge is also possible, however it will be... annoying. The methods implement a $offset = 0 default parameter, if we merge them we'll have something like offsetOfSubstr($substr, $offset = 0, $last = false) or offsetOfSubstr($substr, $last = false, $offset = 0) that will always force you to manually define sometime that usually is automatically done..

Fair point :) I was thinking something along the lines of: indexOf($substr, $offset = 0, $reverse = false) If reverse was true, then it would start at the end of the string.

Basically, I really like the idea of the index method, but I think I'd rather the others live in a project outside the core lib. It's already quite cluttered, though that's my fault. :)

TCB13 added 3 commits May 28, 2015 19:26
…() substringAfterLast() substringBeforeFirst() substringBeforeLast();

Renames: offsetOfSubstr() to indexOf() and offsetOfLastSubstr() oto indexOfLast();
@TCB13
Copy link
Contributor Author

TCB13 commented May 28, 2015

I've made the changes we talked about:

Removed: isEmpty() containsInt() constainsFloat() substringAfterFirst() substringAfterLast() substringBeforeFirst() substringBeforeLast();
Renamed: offsetOfSubstr() to indexOf() and offsetOfLastSubstr() to indexOfLast();

Can you check if it can be merged? Thank you.

@TCB13
Copy link
Contributor Author

TCB13 commented Jun 3, 2015

Hello! Any news on the PR?

I've also created another project with the removed SubString methods, available here: https://github.com/TCB13/SubStringy

Have a nice day ;)

@danielstjules
Copy link
Owner

I've also created another project with the removed SubString methods, available here: https://github.com/TCB13/SubStringy

Thanks! I added it to the links https://github.com/danielstjules/Stringy#links :)

@danielstjules
Copy link
Owner

offsetOfSubstr() to indexOf() and offsetOfLastSubstr() to indexOfLast()

I'm still struggling with the name. It's not really last, but reverse. Do you mind if I pickup your branch?

Thanks a lot for all you work on this btw :)

@TCB13
Copy link
Contributor Author

TCB13 commented Jun 8, 2015

Yeah sure, but please note that SubStringy needs the methods indexOf() and indexOfLast() in order to work, so when you make your mind about the methods, please post here the final names so I can update SubStringy.

Thanks!

@danielstjules
Copy link
Owner

Merged in, thank you for your work on this, and for your patience! I realize I haven't been the most responsive. I came around on the indexOfLast name 😄 I think I might rename the args, but that's about it!

@TCB13
Copy link
Contributor Author

TCB13 commented Jun 29, 2015

Great! Glad to help.

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

Successfully merging this pull request may close these issues.

New substrings and type methods
2 participants