Skip to content

Currency accounts implementation#2

Draft
Sarutoh wants to merge 5 commits intopivorakcourse:mainfrom
Sarutoh:currency-accounts-imlementation
Draft

Currency accounts implementation#2
Sarutoh wants to merge 5 commits intopivorakcourse:mainfrom
Sarutoh:currency-accounts-imlementation

Conversation

@Sarutoh
Copy link
Copy Markdown
Contributor

@Sarutoh Sarutoh commented Sep 23, 2020

No description provided.

Copy link
Copy Markdown
Contributor

@sveredyuk sveredyuk left a comment

Choose a reason for hiding this comment

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

Good job but we have something to improve

@@ -0,0 +1,4 @@
# frozen_string_literal: true

class Euro < Account
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMHO we should allow a user to create own currencies and do not hardcode them

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

okey, i will check how to realize that
and i am sorry for slow response, notifications broken on my phone(

Copy link
Copy Markdown
Contributor Author

@Sarutoh Sarutoh Oct 5, 2020

Choose a reason for hiding this comment

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

I am going to use this gem for exchanging money.
What do you thing about it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Decent solution. Have nothing against it.

create_table :accounts do |t|
t.string :name
t.decimal :balance, default: 0, null: false
t.string :type, default: :UkrainianHryvnia, null: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

t.string :currency_code, default: 'UAH', null: false + index

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.

2 participants