Avoiding boolean traps in es6

Say that we have a ECMAScript6 class like so:

export default class WPAdminLogonPage extends BaseContainer {
	constructor( driver, visit = false ) {
		const jetpackSite = config.get( 'jetpacksite' );
		const wpAdminURL = `https://${jetpackSite}/wp-admin`;
		super( driver, by.css( '#loginform' ), visit, wpAdminURL );
		this.ensureWPAdminStandardLogonShown();
	}

	ensureWPAdminStandardLogonShown() {
		const self = this;
		self.driver.findElement( by.tagName( 'body' ) ).getAttribute( 'class' ).then( ( bodyClasses ) => {
			if ( bodyClasses.indexOf( 'jetpack-sso-form-display' ) > -1 ) {
				return driverHelper.clickWhenClickable( self.driver, by.css( '.jetpack-sso-toggle.wpcom' ) );
			}
		} );
		return self.waitForPage();
	}
}

which we instantiate like so:

this.wpAdminLogonPage = new WPAdminLogonPage( driver, true );

This seems pretty straightforward.

But now imagine we’re adding a new scenario and I have a case where we don’t want to call the ensureWPAdminStandardLogonShown method during constructor.

There’s two ways I see that we could achieve this:

  1. Removing the call to this.ensureWPAdminStandardLogonShown() from the constructor and call this in previous spots where the class is instantiated; or
  2. Adding a second parameter to the constructor that specifies whether the instance should call the ensureWPAdminStandardLogonShown() method

Option 1 is a bit risker as it’s not backwards compatible and we need to make sure all the existing objects are updated to no longer pass the parameter and more importantly explicitly call the ensureWPAdminStandardLogonShown() method on each instance.

Option 2 is ‘safer’ as it’s backwards compatible, so let’s say we just add the second parameter forceStandardLogon and default it to true as that’s what it used to do:

export default class WPAdminLogonPage extends BaseContainer {
	constructor( driver, visit = false, forceStandardLogon = true ) {
		const jetpackSite = config.get( 'jetpacksite' );
		const wpAdminURL = `https://${jetpackSite}/wp-admin`;
		super( driver, by.css( '#loginform' ), visit, wpAdminURL );
		if ( forceStandardLogon === true ) {
			this.ensureWPAdminStandardLogonShown();
		}
	}

	ensureWPAdminStandardLogonShown() {
		const self = this;
		self.driver.findElement( by.tagName( 'body' ) ).getAttribute( 'class' ).then( ( bodyClasses ) => {
			if ( bodyClasses.indexOf( 'jetpack-sso-form-display' ) > -1 ) {
				return driverHelper.clickWhenClickable( self.driver, by.css( '.jetpack-sso-toggle.wpcom' ) );
			}
		} );
		return self.waitForPage();
	}
}

And we can call it like so for our new instance where we don’t want to call the internal method any more:

const wpAdminLogonPage = new WPAdminLogonPage( driver, false, false );

Do you see the issue?

The issue now is that this isn’t very readable code: we’ve got two boolean values one after another and it’s hard to know which one means what. This is known as a boolean trap.

Luckily we’re using ES6 which supports named parameters via ‘destructuring’. This makes our code much clearer to read. Instead of adding just another parameter as a boolean, we use an object:

export default class WPAdminLogonPage extends BaseContainer {
	constructor( driver, visit = false, { forceStandardLogon = true } = {} ) {
		const jetpackSite = config.get( 'jetpacksite' );
		const wpAdminURL = `https://${jetpackSite}/wp-admin`;
		super( driver, by.css( '#loginform' ), visit, wpAdminURL );
		if ( forceStandardLogon === true ) {
			this.ensureWPAdminStandardLogonShown();
		}
	}

	ensureWPAdminStandardLogonShown() {
		const self = this;
		self.driver.findElement( by.tagName( 'body' ) ).getAttribute( 'class' ).then( ( bodyClasses ) => {
			if ( bodyClasses.indexOf( 'jetpack-sso-form-display' ) > -1 ) {
				return driverHelper.clickWhenClickable( self.driver, by.css( '.jetpack-sso-toggle.wpcom' ) );
			}
		} );
		return self.waitForPage();
	}
}

This means it’s much clearer to call, as we can clearly see what the second value is:

const wpAdminLogonPage = new WPAdminLogonPage( driver, false, { forceStandardLogon: false } );

If we didn’t want to worry about backwards compatibility we could also move the visit into the object:

export default class WPAdminLogonPage extends BaseContainer {
	constructor( driver, { visit = false,  forceStandardLogon = true } = {} ) {
		const jetpackSite = config.get( 'jetpacksite' );
		const wpAdminURL = `https://${jetpackSite}/wp-admin`;
		super( driver, by.css( '#loginform' ), visit, wpAdminURL );
		if ( forceStandardLogon === true ) {
			this.ensureWPAdminStandardLogonShown();
		}
	}

	ensureWPAdminStandardLogonShown() {
		const self = this;
		self.driver.findElement( by.tagName( 'body' ) ).getAttribute( 'class' ).then( ( bodyClasses ) => {
			if ( bodyClasses.indexOf( 'jetpack-sso-form-display' ) > -1 ) {
				return driverHelper.clickWhenClickable( self.driver, by.css( '.jetpack-sso-toggle.wpcom' ) );
			}
		} );
		return self.waitForPage();
	}
}

Which gives us by far the easiest to read and understand code:

const wpAdminLogonPage = new WPAdminLogonPage( driver, { visit: false, forceStandardLogon: false } );

What do you think? Have you ever come across a boolean trap like this?

Author: Alister Scott

Alister is an Excellence Wrangler for Automattic.

3 thoughts on “Avoiding boolean traps in es6”

  1. export default class WPAdminLogonPage extends BaseContainer {
        constructor( { driver, visit = false,  forceStandardLogon = true } ) {
            const jetpackSite = config.get( 'jetpacksite' );
            const wpAdminURL = `https://${jetpackSite}/wp-admin`;
            super( driver, by.css( '#loginform' ), visit, wpAdminURL );
            forceStandardLogon === true  ? this.ensureWPAdminStandardLogonShown() : null;
        }
     
        ensureWPAdminStandardLogonShown() {
            const self = this;
            self.driver.findElement( by.tagName( 'body' ) ).getAttribute( 'class' ).then( ( bodyClasses ) => {
                if ( bodyClasses.indexOf( 'jetpack-sso-form-display' ) > -1 ) {
                    return driverHelper.clickWhenClickable( self.driver, by.css( '.jetpack-sso-toggle.wpcom' ) );
                }
            } );
            return self.waitForPage();
        }
    }
    
    const wpAdminLogonPage = new WPAdminLogonPage( { driver, forceStandardLogon: false } ); // to disable forceStandardLogon
    or
    const wpAdminLogonPage = new WPAdminLogonPage( { driver} ); // to enable forceStandardLogon
    

    Like

    1. I wouldn’t tend to put the non-boolean required values like driver into the object. I’d just default to whatever makes sense and let the calling method override it. You can still call the method with the object even with the default value if you wish to be clear.

      Like

Comments are closed.