Skip to content

Add compute value of an expression analyzer#312

Merged
carloscds merged 1 commit intocode-cracker:masterfrom
giggio:compute-expressions
Apr 17, 2015
Merged

Add compute value of an expression analyzer#312
carloscds merged 1 commit intocode-cracker:masterfrom
giggio:compute-expressions

Conversation

@giggio
Copy link
Copy Markdown
Member

@giggio giggio commented Apr 16, 2015

Closes #117

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0%) to 89.82% when pulling c256b25 on giggio:compute-expressions into aa9ea52 on code-cracker:master.

@carloscds carloscds self-assigned this Apr 16, 2015
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@giggio This line is produce an error in value because of culture. Result value is 2.4 and after result.Value.ToString() is 2,4. and two tests fail.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@carloscds I have updated it, but I can't test it, because my system is in en-US. Can you please verify?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@giggio Perfect now!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nice! @carloscds @ElemarJR Do you have any idea on how we could add a unit test to verify this?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@giggio @ElemarJR Do you talk about check culture ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@giggio @ElemarJR It's possible to create a test that check culture, but in this case we will force user to change your culture to en-us or put an alert (inconclusive test) about this ? Maybe put something in documentation about tests using numeric conversions. What do you think ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@carloscds @ElemarJR I am not sure how to solve this. I created issue #313 so we can discuss there.

@giggio giggio force-pushed the compute-expressions branch from c256b25 to 4e43d33 Compare April 16, 2015 13:51
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0%) to 89.82% when pulling 4e43d33 on giggio:compute-expressions into aa9ea52 on code-cracker:master.

carloscds pushed a commit that referenced this pull request Apr 17, 2015
Add compute value of an expression analyzer
@carloscds carloscds merged commit 2db577c into code-cracker:master Apr 17, 2015
@giggio giggio deleted the compute-expressions branch April 17, 2015 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compute value of an expression and replaces it whenever it's possible

3 participants