urlSlice can handle urls with both query params and hash strings

[jgarciabengochea]

My idea for implementation is on finding a query params identifier we will search from the next index to the end of the string looking for a fragment identifier. If the index of the fragment Identifier is greater than the index of the params Identifier (I realize this is redundant now because we only search the substring after the params identifier, my initial implementation was to check if it was greater than 0 ie. we found it) then we will create a hashStr from the fragment identifier to the end of the string and create a queryStr from the index of the params identifier to the fragment Identifier. If the fragment Identifier is not found after the query params Identifier we set the fragment identifier to be the length of the string and create a substring from that. I have included tests for all of the cases that you showed me in your examples. The last thing I changed is I saw that .substr() has been deprecated in favor of .substring() which did not impact the implementation of the code I inherited. Thank you for allowing me to have a chance at my first open source contribution! Any feedback or review is welcome!

[coveralls]

Pull Request Test Coverage Report for Build 76

• 14 of 14 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.05%) to 95.591%

---

Totals
Change from base Build 75:	0.05%
Covered Lines:	416
Relevant Lines:	427

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 79

• 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.02%) to 95.563%

---

Totals
Change from base Build 75:	0.02%
Covered Lines:	412
Relevant Lines:	423

---

💛 - Coveralls

[amitguptagwl]

long paragraph ;) will read in last :)

[jgarciabengochea]

@amitguptagwl just for clarification what do you mean when you say commented changes and to assert query str?

[amitguptagwl]

I've reviews your changes and commented on 2 places for the change

1. "It seems a duplicate test."
2. "Please assert query str as well"

Please do commented changes. In addition, though I'm not sure if a valid fragment string can have '?' but your code may fail if it has. Can you please check?

Example: /test/url#fragment?string

[amitguptagwl]

@jgarciabengochea were you able to understand my last comment? Let me know if there is any confusion.

[jgarciabengochea]

@amitguptagwl Yes, I realized I had read the statement wrong, I understood you wanted me to make the changes you commented on, not make comments with the changes I made, my mistake. I have made the appropriate changes and asserted the test case you proposed.

[amitguptagwl]

Great! There is one last issue. '?' and ';' both are query string identifier. Please check the last else if block

[amitguptagwl]

@jgarciabengochea I hope you are on it.

[jgarciabengochea]

@amitguptagwl Yes! I am on it, sorry for the delay. So the second query string identifier also needs to be able to handle fragments as well, or the last else if block needs to be tested?

[amitguptagwl]

EVerything would be the same. Only the change is when you're checking for a char to be ?, you will have to check for both ? or ;. And remove last else block.

if ( url[i] === '?' || url[i] === ';') {
//..
}

[jgarciabengochea]

Ok great, that's what I assumed you meant but wanted to confirm. Submitted the changes.

[amitguptagwl]

Thanks @jgarciabengochea for your first contribution to this repo. As you said this is your first contribution to any open source project, I would like to say congrats.

To continue your journey of supporting open source projects You may join Team

[jgarciabengochea]

@amitguptagwl Thank you so much for guiding me through this process! I appreciate the opportunity it was a great learning experience!