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

Cleanup DAL system #40

Closed
justinmeiners opened this issue Jan 13, 2016 · 4 comments
Closed

Cleanup DAL system #40

justinmeiners opened this issue Jan 13, 2016 · 4 comments
Labels
enhancement v3 Related to GRA version 3

Comments

@justinmeiners
Copy link
Collaborator

In several files I noticed that many methods have a large amount of code duplicated between them for reading and writing data to/from SQL queries. Could all these be condensed into one function for reading from and one for writing to a SQL query? I am aware that sometimes their are slight design inconsistencies that make this difficult, but from my file diffs they do look very similar.

Here are a few examples:
Patron.cs

  • public static Patron GetObjectByUsername(string logon);
  • public bool Fetch(int PID);
  • public static Patron FetchObject(int PID);
  • public static Patron GetObjectByEmail(string email);

Badge.cs

  • public static int Insert(Badge o);
  • public static int Update(Badge o);

Badge.cs

  • public static Badge GetBadge(int BID);
  • public bool Fetch(int BID);
  • public static Badge FetchObject(int BID);
@k7hpn
Copy link
Member

k7hpn commented Jan 13, 2016

I have a lot of issues with the current manual DAL model in the GRA code (which I suspect was initially generated from a tool anyways). One of my "big picture" task items is to refactor this aspect of the application completely and move to a more integrated ORM-type solution (I'm leaning towards Microsoft's Entity Framework). In such a move, all of this boilerplate DAL code (and the stored procedures) would be eliminated and replaced with a simpler and cleaner data model.

I think that's the right approach though it's not something I'm ready to tackle just yet since I know we have some other changes which take the priority here. I'm envisioning this more as a change for the next major release.

@justinmeiners
Copy link
Collaborator Author

Ok, that sounds like a good plan. If I improved some of the current code by removing duplication (in a fork of your branch) would that be helpful in the short term?

@k7hpn
Copy link
Member

k7hpn commented Jan 14, 2016

You could but I'm not sure that it's worth it - I don't think the duplicated code is causing much of a performance issue.

@justinmeiners justinmeiners changed the title Duplicated Code in Patron Request Theories Duplicated Code in Patron Request Queries Jan 14, 2016
@justinmeiners
Copy link
Collaborator Author

Ok I won't worry about it then. I wasn't thinking of it in terms of performance but maintainability and modification. I will probably be adding a few fields so it could be error prone to copy-paste often.

@justinmeiners justinmeiners changed the title Duplicated Code in Patron Request Queries Cleanup DAL system Jun 18, 2016
@k7hpn k7hpn added the v3 Related to GRA version 3 label Apr 10, 2018
@k7hpn k7hpn closed this as completed Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement v3 Related to GRA version 3
Projects
None yet
Development

No branches or pull requests

2 participants