Skip to content

Conversation

@ConalCosgrove
Copy link
Contributor

Code change in rooms to allow for more granular roomType selection based on provided url parameters

@coveralls
Copy link

coveralls commented Nov 1, 2019

Coverage Status

Coverage increased (+0.003%) to 88.782% when pulling 424dec9 on dynamic-roomType-selection into a70a20d on master.

for (let i = 0; i < urlParamNames.length; i++){
const paramName = urlParamNames[i];
const urlParamValue = urlParams[paramName];
const newRoomType = urlParamValue && _roomTypeMapping[paramName] &&

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this returning a string or a boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A string - if it exists at the index

if (!_defaultRoomType) {
return '';
}
const urlParamNames = Object.keys(urlParams);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason why not using forEach from here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can't break out of a for each, so you'd have to loop through the entire array and then return the found value. This way we can exit the loop as soon as a match is found

@ConalCosgrove ConalCosgrove merged commit 387e8f3 into master Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants