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;

    }
}
\$\endgroup\$
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.

\$\endgroup\$
  • \$\begingroup\$ Plus those queries can be compiled! \$\endgroup\$ – IAbstract Sep 14 '11 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\$ – Nathanael Sep 14 '11 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

\$\endgroup\$
  • \$\begingroup\$ Never forget to mention table valued functions (parametized views)! ; ) \$\endgroup\$ – Casper Leon Nielsen Jan 10 '13 at 0:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.