Last active
February 23, 2020 17:34
-
-
Save Integralist/7702823 to your computer and use it in GitHub Desktop.
Revisions
-
Integralist revised this gist
Nov 29, 2013 . 1 changed file with 1 addition and 1 deletion.There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -16,6 +16,6 @@ if (enhancedExperience) { One thing to be aware of is: refactoring can actually be a bad thing if you don't know why you're doing it. If you're just blindly following predefined rules of refactoring then the above small tweak could end up making the code harder to read. Imagine if we were in a situation where there wasn't a logical association between `modernDevice` and `holepunched`. We could end up creating a variable like `passesOurChecks` (this is a poor example I know). Which would mean this new variable would actually make the code slightly harder to read because it wouldn't be descriptive enough and would likely mean the reader would need to check what the value of that variable was to better understand the code. Just something to keep in mind. -
Integralist revised this gist
Nov 29, 2013 . 2 changed files with 0 additions and 0 deletions.There are no files selected for viewing
File renamed without changes.File renamed without changes. -
Integralist revised this gist
Nov 29, 2013 . 1 changed file with 1 addition and 1 deletion.There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -4,5 +4,5 @@ I refactored this code by making just a couple of very basic tweaks... 2. removed unnecessary syntax cruft from `isProxyBrowser` 3. give the variables descriptive names 4. further break down the logic (e.g. group the checks for 'cutting the mustard' and 'being a proxy based browser') 5. reversed the `isProxyBased` logic so it becomes `isNotProxyBased` (this meant we didn't clutter `modernDevice` with extra syntax and made it as readable as possible) 6. use selected variables within the conditional -
Integralist revised this gist
Nov 29, 2013 . 4 changed files with 0 additions and 0 deletions.There are no files selected for viewing
File renamed without changes.File renamed without changes.File renamed without changes.File renamed without changes. -
Integralist revised this gist
Nov 29, 2013 . 3 changed files with 24 additions and 3 deletions.There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -2,4 +2,4 @@ var isProxyBased = (/S40[\w]{3,5}Browser|Opera\sMini\//i).test(navigator.userAge if (('querySelector' in document && 'localStorage' in window && 'addEventListener' in window && !isProxyBased) || (isIE > 6 && document.getElementById('js-holepunched'))) { // do stuff } This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -3,6 +3,6 @@ var cutsTheMustard = 'querySelector' in document && 'localStorage' in window && modernDevice = cutsTheMustard && isNotProxyBased, holepunched = isIE > 6 && document.getElementById('js-holepunched'); if (modernDevice || holepunched) { // do stuff } This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -0,0 +1,21 @@ Is there any thing more we could do with this code to make it as simple as possible to understand? Well, one small tweak we *could* make would be to simplify the conditional even further. We could do this by creating another variable (again with a helpfully descriptive identifier) to hold the check of `modernDevice || holepunched`, like so... ```js var cutsTheMustard = 'querySelector' in document && 'localStorage' in window && 'addEventListener' in window, isNotProxyBased = !/S40[\w]{3,5}Browser|Opera\sMini\//i.test(navigator.userAgent), modernDevice = cutsTheMustard && isNotProxyBased, holepunched = isIE > 6 && document.getElementById('js-holepunched'), enhancedExperience = modernDevice || holepunched; if (enhancedExperience) { // do stuff } ``` One thing to be aware of is: refactoring can actually be a bad thing if you don't know why you're doing it. If you're just blindly following predefined rules of refactoring then the above small tweak could end up making the code harder to read. Imagine if we were in a situation where there wasn't a logic association between `modernDevice` and `holepunched`. We could end up creating a variable like `passesOurChecks` (this is a poor example I know). Which would mean this new variable would actually make the code slightly harder to read because it wouldn't be descriptive enough and would likely mean the reader would need to check what the value of that variable was to better understand the code. Just something to keep in mind. -
Integralist created this gist
Nov 29, 2013 .There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -0,0 +1,5 @@ var isProxyBased = (/S40[\w]{3,5}Browser|Opera\sMini\//i).test(navigator.userAgent); if (('querySelector' in document && 'localStorage' in window && 'addEventListener' in window && !isProxyBased) || (isIE > 6 && document.getElementById('js-holepunched'))) { // do stuff } This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -0,0 +1,8 @@ I refactored this code by making just a couple of very basic tweaks... 1. move all logic out of the conditional and into variables 2. removed unnecessary syntax cruft from `isProxyBrowser` 3. give the variables descriptive names 4. further break down the logic (e.g. group the checks for 'cutting the mustard' and 'being a proxy based browser') 5. reversed the `isProxyBrowser` logic so it becomes `isNotProxyBased` (this meant we didn't clutter `modernDevice` with extra syntax and made it as readable as possible) 6. use selected variables within the conditional This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode charactersOriginal file line number Diff line number Diff line change @@ -0,0 +1,8 @@ var cutsTheMustard = 'querySelector' in document && 'localStorage' in window && 'addEventListener' in window, isNotProxyBased = !/S40[\w]{3,5}Browser|Opera\sMini\//i.test(navigator.userAgent), modernDevice = cutsTheMustard && isNotProxyBased, holepunched = isIE > 6 && document.getElementById('js-holepunched'); if (modernDevice || holepunched) { // do stuff }