Skip to content
This repository has been archived by the owner on Apr 1, 2019. It is now read-only.

Addressing early review comments from Marcos. #37

Merged
merged 3 commits into from
Aug 27, 2015

Conversation

emtwo
Copy link
Contributor

@emtwo emtwo commented Aug 21, 2015

No description provided.

@@ -22,13 +22,13 @@
return this.bottom - this.top;
},
set width(v) {
this.right = this.left + v;
this.right = this.left + Number(v);
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

var num = Number.parseInt(v, 10);
Number.isNaN(num){
  return;
}
this.right = this.left + num;

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should have a generic type enforcement for Rect that throws if any of the values is NaN. Also, there is already a Rect class on the Window object it seems (that is different from this one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make the change in your first comment and file an issue for generic type enforcement and changing the name of Rect to something else. How does that sound?

@emtwo
Copy link
Contributor Author

emtwo commented Aug 27, 2015

r? @marcoscaceres

@marcoscaceres
Copy link
Contributor

r+

emtwo pushed a commit that referenced this pull request Aug 27, 2015
Addressing early review comments from Marcos.
@emtwo emtwo merged commit 550b7c2 into gh-pages Aug 27, 2015
@marcoscaceres marcoscaceres deleted the emtwo/address-comments branch August 27, 2015 19:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants