Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #2749287: Implement a Calculator class and use it in the Price value object #408

Closed
wants to merge 1 commit into from

Conversation

FrancisBaileyH
Copy link

No description provided.

@bojanz
Copy link
Contributor

bojanz commented Jun 18, 2016

Thanks! Do you mind filling out the docblocks? We need a description for each method, param, return value.

@FrancisBaileyH
Copy link
Author

@bojanz I've gone ahead and filled out the doc blocks.

@FrancisBaileyH
Copy link
Author

Fixed two docblocks with improper spacing.

* This exception is thrown when attempting to perform math using two different currencies.
* For example, adding a price in USD to another in EUR.
*/
class CurrencyMismatchException extends \RuntimeException {
Copy link
Contributor

Choose a reason for hiding this comment

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

@bojanz should we have an Exception namespace for these?

Copy link
Author

Choose a reason for hiding this comment

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

That's an easy change so let me know if this is something you'd like implemented.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we should. Follows the example of other Exception classes in Commerce.

@FrancisBaileyH
Copy link
Author

@bojanz @mglaman any updates on this pr?

* @param string $rate
* A currency rate corresponding to the currency code.
*
* @return Price
Copy link
Contributor

Choose a reason for hiding this comment

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

Any object name given as a return or param hint must be fully qualified (have the full namespace).

@FrancisBaileyH
Copy link
Author

@bojanz okay I applied your suggested changes.

@bojanz
Copy link
Contributor

bojanz commented Aug 24, 2016

Committed in 071d9ba after some bike shedding :)

@bojanz bojanz closed this Aug 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants