Category Archives: Coding

Coding

Code Reviews on Closed Pull Requests

1 Comment

Good Code Review (CR) on Pull Request (PR) are at the heart creating high quality code. Code reviews on closed pull requests can feel awkward.

I believe we should welcome feedback on all PRs — even closed ones.

A coworker of mine came back from a short vacation and read over my PRs I had opened and merged while they were out. They spotted a bug in one of my PRs and they left a comment pointing it out. They however felt deeply guilty commenting on a closed PR and apologized to me. This is odd to me.

Merged PRs often get treated as code where the “Ship that has sailed”. Treating merged code as “untouchable” forgoes the ownership and responsibility we developers have for the code we create.

I suggest a cultural shift: To strive to produce high quality code, we ought to welcome and be open to Code Review feedback, before, during, and after the life of a Pull Request.

How we allocate time to address the feedback will range from team to team but could be new backlogged stories, re-opening a new PR, or just answering questions.

How do you feel about code reviews on closed pull requests?
Is there a better way to handle code feedback after it’s merged?

Coding

Using Immutable.js .update() instead of .get() then .set()

Leave a comment

Often you need to deeply update an complex Map or List. Using Immutable.js .update() is a powerful way to this kind of update.

Given this state

const = myState = Immutable.Map({
'123': Immutable.Map({
id: 123,
b: false,
}),
'456': Immutable.Map({
id: 456,
b: true,
}),
});

Don’t use .get() then .set()

const myUpdatedItem = myState.get('123').set('b', true);
const myNewState = myState.set('123', myUpdatedItem);

Using .get() then .set() seems simple and clean, but it’s slow, it fully replaced the the current Map. While the results are the same, ImmutableJS cannot optimize against it and cannot make some faster assumptions.

Using Immutable.js .update()

const myNewState = myState.update('123', (myMap) => {
return myMap.set('b', true);
});

It is much better to using Immutable.js .update(). Immutable can keep more references and make assumptions that allow it to optimize the update. For more, check the .update() docs.

You can also use .updateIn() for deeply nested data. Read my post on Immutable get() vs .getIn() to see how it works.

Coding

Exclude redirecting subdomains in htaccess RewriteRule

Leave a comment

Redirecting websites to be secure with https can be a little tricky.  I had to exclude redirecting subdomains in htaccess RewriteRule.  This mostly because a number of my subdomain’s code lives within a subfolder of my primary domain.

How to Exclude redirecting subdomains in htaccess RewriteRule

To exclude redirecting subdomains in htaccess RewriteRule use the following RewriteCond. This tell Apache to only use the redirect if the request is to the top level domain, not a subdomain.

RewriteCond %{HTTP_HOST} ^[^.]+\.[^.]+$

For instance, my .htaccess for jstassen.com looks like:

RewriteEngine On
RewriteCond %{SERVER_PORT} 80
RewriteCond %{HTTP_HOST} ^[^.]+\.[^.]+$
RewriteRule ^(.*)$ https://jstassen.com/$1 [R=301,L]

This regex only fully matches the base domain. You can see this on Debuggex.

Exclude redirecting subdomains in htaccess RewriteRule

Note: websites with www are considered a subdomain. For instance www.jstassen.com is a subdomain of jstassen.com just like blog.jstasseen.com. This rule will therefore exclude the www subdomain. I have my www subdomain auto redirect to remove the www which I recommend.

Coding

Comparing Immutable.Set() and Immutable.OrderedSet()

Leave a comment

When comparing Immutable.Set() and Immutable.OrderedSet() you’ll need to convert the OderedSet down to just a plain Set.

Comparing Immutable.Set() and Immutable.OrderedSet()

To compare equality with a Set and OrderedSet use this:

mySet.equals( myOrderedSet.toSet() )

Immutable .equals() compares the left and right to decide if they are both Ordered. If one is ordered and the other is not, they are not equal. isOrdered(a) !== isOrdered(b).

My Feelings

I find this a bit odd, especially when the left is a plain Set. I feel regardless if the right is Ordered or not, it’s the contents that we’re interested in comparing. However this is the Immutable.js definition. You will need to downgraded the OrderedSet for comparison via .toSet() which does have a slight cost to it. Immutable performance is worth keeping an eye on.

Coding

Changing SourceTree’s Default Remote

Leave a comment

I’m going to talk about changing SourceTree’s default remote.

When you’re pushing a new branch, SourceTree will automatically guess which remote you wish to push to by default.

SourceTree defaults to origin

By default SourceTree will always pick origin to push a new branch to. Depending on your workflow, you may want to set another remote as your default, let’s say my-fork for instance.

SourceTree defaults pushing new branches to origin

Renaming a remote

Interestingly all we have to do is rename our remotes. Since SourceTree always picks origin we just need to rename to anything else. Perhaps to the name of owner of the repo like SproutSocial or FaceBook. It’s safe to rename the remotes, it’s just a nickname only used locally.

(I’ve never been a fan of “origin“, it’s terribly confusing and ambiguous for those learning git).

Steps to rename the remote

via git

git remote rename origin SproutSocial

In SourceTree

  • Open the repo settings (gear in top right)
  • Remotes tab
  • Click on the remote
  • Edit
  • Change the Remote Name

SourceTree Remotes Setting
Editing remote names

Changing SourceTree’s Default Remote

Once we’ve renamed origin, SourceTree will always pick the remote that comes first alphabetically. So, do the ‘ol classic prefixed by numbers. Here I’ll name them 01-my-fork and 02-origin.

Boom! SourceTree picks 01-my-fork by default!

Changing SourceTree's Default Remote
Setting your preferred remote as default

Simple workflow optimizations like this, make me happy.

Coding

Pagination naming conventions

Leave a comment

I’ve been working with some some APIs that have some different patterns for pagination. Which got me to thinking about naming an pagination conventions.

The Problem with ‘up’ and ‘down’ naming

Namely, when describing pagination actions I think it’s probably best to avoid ‘up’ and ‘down’ for naming. These are descriptive words for the ui, not so much of the data.

A -> Z

That is ‘up’ could potentially mean different things in the data depending one what you’re presenting. Let’s say you sort you a list A -> Z by default.  What does down look like in both of these situations. ‘down’ in our minds may imply ‘down’ the alphabet towards the end. But if we can sort to Z -> A the idea of down becomes ambiguous and a bit confusing. For instance if we’re sitting on page M and say down which way does that mean?

Newsted -> Oldest

Let’s consider something that has more chronology. Perhaps a list of recent transactions sorted by date. We could safely presume as you scroll down you retrieve older and older transactions. Describing the each page as down seems reasonable, we’re paging down in time. Specifically the unix epoch timestamp is a number that literally goes down as you go back in time. 1508480500 -> 1508480000.  Makes sense.

Now let’s flip our transaction order to be Oldest first. Now we go 1508480000 -> 1508480500 when paginating down.  Paging down make the number go up. That can get confusing.

Scroll down -> Ticker

To emphasis the opinion of up and down being a UI concern and description. Let’s try one last scenario.

Let’s say we a have a simple twitter feed where we render out messages. Let’s say it’s your typical feed that scrolls down along the page. Paging down make sense here.

But let’s say we introduce stock ticker-styled feed, well now down doesn’t make as much sense. right means down.

Punchline: next and previous

Paginate via a term that is more generic like next and previous seems much cleaner regardless of the UI rendering style.

  • They don’t describe the UI directly.
  • They don’t per-say describe the direction of sorting.
  • Describe the data intent of getting the next chunk.
  • They are ambiguous enough that next don’t imply direction but rather intent.

Pagination is hard

Pagination naming is hard, but good design is amazing. These are just some of my observation and thoughts after working with different apps and APIs. Who knows, maybe there is something better!

Coding

Immutable.js .get() vs. .getIn()

1 Comment

At Sprout Social in some areas of our frontend app we use Immutable.js for our Redux store.

Standardizing Selector Styles.

When selecting state out of our store we’ve written a collection of selectors to consolidate selectors logic. We’ve always write these selectors in array notation to keep styles consistent.

messageStore.getIn([id, 'author', 'screenname']);

With all selectors written in array notation, for Immutable.js we use .getIn() by default — regardless if the path is only one key deep. It’s very convenient. Keeps our selectors looking consistent in shape.

messageStore.getIn([id]);

Immutable.js .get() vs. .getIn()

However it is always faster to do a .get() instead of a .getIn() .  For  .getIn() Immutable.js has to iterate through an array path and check the result for each key path along the way. This therefore makes .getIn() expensive and .get() ultimately cheaper.

Abstract .getIn()

So if we wish to keep all selectors a consistent array path style, but take advantage of the speed of.get() whenever possible we could make a get abstraction. The first thing to do is to create an abstraction for Immutable .getIn() — something like this:

Add .get() to the abstraction.

How could we get this handy abstraction to take advantage of .get() whenever possible?
We could peak into the arrayPath , if there is only one value, use plain old .get(). Seems simple enough.

But is it performant?

Array length checking is cheap. With a quick little JSPerf test we can see immediately takins the time to check if we should use  .get() is more than twice as fast than just always using .getIn().

https://jsperf.com/immutable-get-getin-path-check/1

In the end

In the end, this is a micro optimization. However, we saw ~50ms to ~150ms speedups for every actions in the app. It really depends on the how often your selectors run. We have a large number of selectors that fire quite a bit. For such a small change, we’ll gladly take any performance boost over 100ms.

Don’t worry about the abstraction, just remember to prefer.get() over .getIn() whenever possible.

Pro tip: Same goes for .set() / .setIn() and .update() / .updateIn(), etc.

Coding

React “stateProps PrecalculationError” and TypeError ‘state’ of undefined

Leave a comment

Debugging a React component can be a pain.

There are many causes for “statePropsPrecalculationError”, for debugging see my post on Debugging React statePropsPrecalculationError post.

When you specifically have ” ‘state’ of undefined” the fix is simple:

Recently I ran into “statePropsPrecalculationError” alongside with the the console warning TypeError: Cannot read property ‘state’ of undefined(…)`.

This is super easy to fix, but not immediately obvious. Whenever you use `this.setState()` inside your component, you must define your initial state!

Just add a `getInitialState()` definition!


const MyComponent = React.createClass({
getInitialState() {
return {
isOpen: false
};
},
toggleOpen() {
this.setState({isOpen: !this.state.isOpen});
},
render() {
return (

Hello World!

);
}
});

Coding

Code: Regex for Instagram Username and Hashtags

6 Comments

Instagram is unique when it comes to the rules for usernames and hashtags. Here is a regex to capture and validating them with the JavaScript regex engine.

The rules

  • Two matches @ mentions with no space between @thebox193@discodude
  • Matches with one . in them @disco.dude but not two .. @disco..dude
  • Beginning period not matched @.discodude
  • Ending period not matched @discodude.
  • Match underscores _ @_disco__dude_
  • Max characters of 30 @1234567890123456789012345678901234567890

The RegEx

(?:@)([A-Za-z0-9_](?:(?:[A-Za-z0-9_]|(?:\.(?!\.))){0,28}(?:[A-Za-z0-9_]))?)
Instagram username RegEx

Hashtags

For hashtags use the same regex, it’s the same rules, just replace the @ for a #.

(?:#)([A-Za-z0-9_](?:(?:[A-Za-z0-9_]|(?:\.(?!\.))){0,28}(?:[A-Za-z0-9_]))?)

Examples

You can try it out over on Debuggex. Here is a an example implemented in JavaScript.

Coding

LoDash.js / Underscore.js mixins

Leave a comment

Here are a few mixins I’ve created for LoDash.js I’ve found handy.

_.getDeep

/**
* Safely retrieves the value of a deeply nested object value
* @param {object} O The Object to be searched
* @param {string} str Dot notation string of the address of the attribute to be retrieved
* @return {mixed} Value of nested item or undefined
* @link http://stackoverflow.com/a/15400575/417822
*/
_.mixin({'getDeep': function(O, str) {
if ( !_.isObject(O) ) return;

var seg= str.split('.');
while(O && seg.length) {
O = O[seg.shift()];
}
return O;
}});

_.hasDeep

/**
* Tests if a deep value has a value/exists. Uses _.getDeep to retrieve value.
* @param {object} O The Object to be tested
* @param {string} str Dot notation string of the address of the attributed to be tested
* @return {boolean}
* @todo validate that falesy values 'false' '0' etc come back as true.
*/
_.mixin({'hasDeep': function(O, str) {
return !_.isUndefined( _.getDeep(O, str) );
}});

_.isDefined
Because typing !_.isUndefined() all the time is tiresome. Using _.isDefined is clear and more readable.

/**
* Shorthand for !_.isUndefined
* @param {mixed} e Item to be tested
* @return {boolean}
*/
_.mixin({'isDefined': function(e) {
return !_.isUndefined(e);
}});

_.compactObject

/**
* Removes all falsy and undefined values from an object
* @param {object} o Object to be compacted
* @return {object} Compact Object
* @link http://stackoverflow.com/a/14058408/417822
*/
_.mixin({'compactObject': function(o) {
var clone = _.clone(o);

_.each(clone, function(v, k) {
if(!v) delete clone[k];
});

return clone;
}});

_.compactObject

/**
* Toggles an array element
* @param {Array} arr Source Array
* @param {Mixed} val Value to toggle
* @param {Boolean} Optional: Boolean to determine whether to add or remove
* @return {Array} Result
*/
_.mixin({'arrayToggle': function(arr, val, state) {
if ( _.indexOf(arr,val)==-1 || state)
return _.union(arr,[val]);
else
return _.without(arr,val);
}});