Do more testings and fix some problems

[peter279k]

Firstly, thank you your easy PHP package to manipulate the URL!
Here are some request changes that you need to consider.

Changed log

• do more testings.
• change the addBackslash method to addBackSlash method because the black and slash are the separated words.
• original switch...case usage is weird because it should not directly use return in switch block.
  And it should add the default case then the return will out of the switch block.
  The default case I use return false to handle the unexpected cases.
  Perhaps we can use the InvalidArgumentException to catch this.
• The fix command in composer.json will output the ERROR: The file "src,tests" does not exist. message.
  It seems that the phpcbf identifies a file, not directories. We should change this to the vendor/bin/phpcbf src tests command.

Please review this.

Thanks.

[codecov-io]

Codecov Report

Merging #1 into master will increase coverage by 4.97%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master       #1      +/-   ##
============================================
+ Coverage      88.5%   93.47%   +4.97%     
  Complexity       41       41              
============================================
  Files             1        1              
  Lines            87       92       +5     
============================================
+ Hits             77       86       +9     
+ Misses           10        6       -4

Impacted Files	Coverage Δ	Complexity Δ
src/Url.php	93.47% <100%> (+4.97%)	41 <6> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3659c9...240fb80. Read the comment docs.

[josantonius]

Hello Peter,

Great, I think that those fixes are perfect, although I am curious about switch... case... Why would you advise against the use of return? Readability? Contrary to some standard? Officially, there is no mention of the possibility of using it, but some threads do not think it as a problem.

I recently thought about refactoring the addBackSlash() method to something like this:

public static function addBackSlash($uri, $position = 'end')
{
    switch ($position) {
        case 'top':
            $uri = '/' . ltrim($uri . '/');
            break;
        case 'end':
            $uri = rtrim($uri . '/') . '/';
            break;
        case 'both':
            $uri = '/' . trim($uri . '/') . '/';
            break;
        default:
            $uri = false;
    }
    return $uri;
}

It would be faster, although it would have a slightly different result:

addBackSlash('sample///',' both');

Before: '/sample///'
After: '/sample/'

This should not be a problem for URLs, except for: addBackSlash('http://', 'end'):

Before: 'http://'
After: 'http:/'

What do you think?

Thanks!

[peter279k]

Hi @josantonius, thank you for your reply.

• I think the new return usage is readability.
• I think the new code refactoring is about the addBlackSlach method is fine.
  I think we should add some tips in README.md after using this new approach.

[josantonius]

Perfect.

Thank you again!