search

DuckMan435 / PortfolioViewer

0 stars 0 forks source link

SecurityModel better approach? #1

Open apryiomka opened 8 years ago

apryiomka commented 8 years ago

The SecurityModel has properties such as BondInterestRate which are specific to Bond securities as well as FundDividend which would be a Fund or a Stock? Do you think there is a better approach to define the Models (Entities)?

DuckMan435 commented 8 years ago

A better approach would be to make a SecurityModel base class that Stock, Fund and Bond Security Models can derive from. The base class would most likely very bare bones because the amount of common properties among all 3 securities is relatively small.

apryiomka commented 8 years ago

Yes, same would, probably, translate into the DB schema (using entity framework), as result you would have three tables with each security type. Also, do you think we can improve the PortfolioModel. I assumed you didn't use any FK constraints for the sake of the assessment, but I have noticed you use UserName to link the portfolio to the user. Do you think we can have a better way to link users with Portfolios?

DuckMan435 commented 8 years ago

Yes, I've thought about this as well I would probably change the link by creating a Client table, which the entries would be tied to the Portfolio. Each user would be tied to a client, this idea is somewhat related to the "bonus" task of allowing admin users (I'm assuming admin of a client), so that you can have multiple users tied to 1 Portfolio. With the current structure that is not currently possible.

apryiomka commented 8 years ago

I was mostly referring to the FK relationship. It would be a good idea to have user ID as a foreign key rather than username in the PortfolioModel table. If you have a Users table it is a good idea to build your relational tables using primary keys as foreign keys, unless you have a reason not to (sometimes it would be very much the case). Besides, if the ID is an INT type, it would also increase the performance vs using strings like emails or user names. On the other hand, if you have a username as a foreign key, you don't have to do joins with the users table to pull the user's profiles if you query by username. But that can be addressed by pulling profiles using user's id vs username.

DuckMan435 commented 8 years ago

@apryiomka Working through this, I see a couple comments with implementing the above suggestions. I created inheritance using the SecurityModel as a base (StockModel, FundModel, BondModel), but doing some research I have noticed that it is suggested to use table-per-hierarchy approach instead of table-per-type approach as mentioned above.

As for UserName as FK issue, currently I am using the asp.net identity which establishes Id for a user to be a string (in Guid form) by default instead of an int. Were you looking for me to disconnect from the asp.net identity entirely or perhaps change how the asp.net identity User Id is defined?

apryiomka commented 8 years ago

Since you have different types of the SecurityModel, do you still need SecurityType enum? Also, how would you change the SecurityModel to "enforce" inheritance (use of the derived types)?

DuckMan435 commented 8 years ago

I would make the SecurityModel abstract and change the SecurityType enum to an abstract string property that I can override in each of the derived classes.

DuckMan435 commented 8 years ago

Updated the SecurityModel to be abstract and required changes