5
\$\begingroup\$

I have two text boxes and one combobox. I am retrieving the data based upon the text entered into the text boxes and combobox selection, by using the following method:

 private void textboxfillnames()
 {
 if (txtlastname.Text != "")
 {
 var totalmembers = from tsgentity in eclipse.members
 join memtomships in eclipse.membertomships on tsgentity.member_Id equals memtomships.member_Id
 join mshipoptiions in eclipse.mshipoptions on memtomships.mshipOption_Id equals mshipoptiions.mshipOption_Id
 join mshiptypes in eclipse.mshiptypes on mshipoptiions.mshipType_Id equals mshiptypes.mshipType_Id
 join mshipstatus in eclipse.mshipstatustypes on memtomships.mshipStatusType_Id equals mshipstatus.mshipStatusType_Id
 where tsgentity.member_Lastname.StartsWith(txtlastname.Text)
 select new {
 tsgentity.member_Id,
 tsgentity.member_Lastname,
 tsgentity.member_Firstname,
 tsgentity.member_Postcode,
 tsgentity.member_Reference,
 tsgentity.member_CardNum,
 tsgentity.member_IsBiometric,
 tsgentity.member_Dob,
 mshiptypes.mshipType_Name,
 mshipstatus.mshipStatusType_Name,
 memtomships.memberToMship_EndDate 
 };
 dgvReportMembers.DataSource = totalmembers;
 }
 if (txtpostcode.Text != "")
 {
 var totalmembers = from tsgentity in eclipse.members
 join memtomships in eclipse.membertomships on tsgentity.member_Id equals memtomships.member_Id
 join mshipoptiions in eclipse.mshipoptions on memtomships.mshipOption_Id equals mshipoptiions.mshipOption_Id
 join mshiptypes in eclipse.mshiptypes on mshipoptiions.mshipType_Id equals mshiptypes.mshipType_Id
 join mshipstatus in eclipse.mshipstatustypes on memtomships.mshipStatusType_Id equals mshipstatus.mshipStatusType_Id
 where tsgentity.member_Postcode.StartsWith(txtpostcode.Text)
 select new {
 tsgentity.member_Id,
 tsgentity.member_Lastname,
 tsgentity.member_Firstname,
 tsgentity.member_Postcode,
 tsgentity.member_Reference,
 tsgentity.member_CardNum,
 tsgentity.member_IsBiometric,
 tsgentity.member_Dob,
 mshiptypes.mshipType_Name,
 mshipstatus.mshipStatusType_Name,
 memtomships.memberToMship_EndDate 
 };
 dgvReportMembers.DataSource = totalmembers; 
 }
 if (cbGEStatustype.Text != null)
 { 
 var totalmembers = from tsgentity in eclipse.members
 join memtomships in eclipse.membertomships on tsgentity.member_Id equals memtomships.member_Id
 join mshipoptiions in eclipse.mshipoptions on memtomships.mshipOption_Id equals mshipoptiions.mshipOption_Id
 join mshiptypes in eclipse.mshiptypes on mshipoptiions.mshipType_Id equals mshiptypes.mshipType_Id
 join mshipstatus in eclipse.mshipstatustypes on memtomships.mshipStatusType_Id equals mshipstatus.mshipStatusType_Id
 where mshipstatus.mshipStatusType_Name.StartsWith(cbGEStatustype.Text)
 select new {
 tsgentity.member_Id,
 tsgentity.member_Lastname,
 tsgentity.member_Firstname,
 tsgentity.member_Postcode,
 tsgentity.member_Reference,
 tsgentity.member_CardNum,
 tsgentity.member_IsBiometric,
 tsgentity.member_Dob,
 mshiptypes.mshipType_Name,
 mshipstatus.mshipStatusType_Name,
 memtomships.memberToMship_EndDate 
 };
 dgvReportMembers.DataSource = totalmembers;
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 14, 2011 at 14:32
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

You should wrap each query into a function that takes in a string and returns your query result. You should not make direct reference to GUI objects in your business logic as you have done.

You should have a controller negotiating the transfer of data between your model (eclipse.members) and your view (the various text and combo boxes you are populating).

EDIT:

I'm not sure what your level of knowledge is, but if all that went over your head let me know and I'll post a small example later today. ;)

Promised Sample Code:

To learn more about why I have structured the code like this, do some searching for Model View Presenter. The Presenter in this sample is the controller I referred to above.

NOTE: You shouldn't name classes and interfaces like this in real life, but I've done it to emphasize which part of the MVP pattern I am implementing.

NOTE: These are not fully defined classes or interfaces. I've only shown the relevant parts to your question.

//
// Presenter
//
public class Presenter
{
 private IView _view;
 private IModel _model;
 public void FindAndDisplayMemberList
 {
 // I have made a few assumptions here. 
 // First, I wasn't sure what dgvReportMembers was. I assumed that it was some
 // sort of view object being used to display the results on a form.
 //
 // Second, I noticed you were assigning it three times. I've assumed that
 // you really only want to assign it once. Therefore once this function finds
 // a valid field, it returns.
 //
 // Finally, the large amount of very similar code here should suggest that you
 // need to rethink your structure. But without more knowledge of what you are
 // building I can't give you much help with that.
 string lastName = _view.LastName;
 if (!String.IsNullOrEmpty(lastName))
 {
 var data = _model.GetMemberListFromLastName(lastName);
 _view.SetReportMembersDataSource(data);
 return;
 }
 string postalCode = _view.PostalCode;
 if (!String.IsNullOrEmpty(postalCode))
 {
 var data = _model.GetMemberListFromPostalCode(postalCode);
 _view.SetReportMembersDataSource(data);
 return;
 }
 string geStatus = _view.SelectedGeStatus;
 if (!String.IsNullOrEmpty(geStatus))
 {
 var data = _model.GetMemberListFromGeStatus(geStatus);
 _view.SetReportMembersDataSource(data);
 return;
 }
 }
}
//
// View Interface - Your form that displays the relevant data should implement this.
//
public interface IView
{
 string LastName { get; set; }
 string PostalCode { get; set; }
 string SelectedGeStatus { get; }
 void SetReportMembersDataSource(object source);
}
//
// Model Interface - Controller uses this to access the database. 
// Your model should not know anything about the
// presenter or the view.
//
public interface IModel
{
 //
 // NOTE: MemberInfo is a class that should hold all of the fields you
 // you are selecting in the query. (Id, LastName, etc.)
 //
 List<MemberInfo> GetMemberListFromLastName(string lastName);
 List<MemberInfo> GetMemberListFromPostalCode(string postalCode);
 List<MemberInfo> GetMemberListFromGeStatus(string geStatus);
}

There are a few improvements to note in this code.

First code related to the view and the model is isolated.

Second, the Presenter works with interfaces, so it never needs to know what the actual model or view object is.

Third, in the view interface, I don't expose the actual GUI controls being used to display data. I use common types such as string to pass data back and forth, and let the view worry about displaying that with an appropriate GUI object.

answered Sep 14, 2011 at 15:46
\$\endgroup\$
2
  • \$\begingroup\$ Plus those queries can be compiled! \$\endgroup\$ Commented Sep 14, 2011 at 15:49
  • \$\begingroup\$ @errorcode105: I did see the joins. Those can be wrapped into the functions I suggested as well. I used the term 'query' to mean your entire request starting with from. (I'm not a SQL expert however so I won't presume to comment on how you could improve that portion of your code.) And yes, I'll put up some example code a little later tonight. \$\endgroup\$ Commented Sep 14, 2011 at 20:10
1
\$\begingroup\$

I generaly don't use LINQ all that much because most of the time, it's not as performant. I use views instead (Database views, not to be mistaken with views from the previous answer ;D) which are created manually, instead of dynamicly at runtime (=LINQ).

In this situation I'm sure it doesn't matter all that much, seeing how you're using "Select new {...});", which in my experience, means that the queries remain relatively "clean".

If you're however using a lot of "Contains()" or other LINQ-commands, LINQ has a hard time creating a good query. If you use an SQL profiler (a tool, I like to use anjlab's) you can check which query gets sent to the database (really good way to see if you should write a query yourself or not).

If you're using Entity Framework you can insert the view into your model, and call it like it's a tabel (from member in objectContext.vwblalbah select member), you can also still use WHERE statements on it if you'd please (where member.memberID == ID)...

If you don't like writing views in your database, you can also pre-generate queries, so that LINQ doesn't have to dynamicly create it. I think (don't know for sure though) that views are still faster.

I hope this helped

answered Sep 15, 2011 at 19:16
\$\endgroup\$
1
  • \$\begingroup\$ Never forget to mention table valued functions (parametized views)! ; ) \$\endgroup\$ Commented Jan 10, 2013 at 0:28

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.