Should this code be in the code behind or separate class?

Question

I'm developing a log-on page where users either select or create an associated organization when they sign up. There can be either 1 user per organization or many users per organization, this is specified at runtime via web.config.

I have a separate project that handles all user account creation. I don't know if this code should be in the Webforms project or the user account creation project since it is driven by the user's selection in the GUI?

    private IpasLogOnManagement.IpasOrganization EnsureOrganizationExists()
    {
        IpasLogOnManagement.IpasOrganization organization = null;

        if (OrganizationsSeparateFromUsers)
        {
            if (lstOrganizations.SelectedIndex == -1)
            {
                try
                {
                    organization = organizationProvider.SearchForOrganizationByName(txtOrganizationName.Text);
                }
                catch (IndexOutOfRangeException)
                {
                    //Organization does not exist, so create it
                    organization = manager.CreateOrganization(txtOrganizationName.Text);
                }
            }
            else
            {
                string selectedName = lstOrganizations.Items[lstOrganizations.SelectedIndex].Text;
                organization = organizationProvider.SearchForOrganizationByName(selectedName);
            }
        }
        else
        {
            organization = manager.CreateOrganization(txtEmail.Text);
        }
        return organization;
    }

+1 Perfect question for the site, which will help inform future developers about where boundaries between modules should be. — Jimmy Hoffa
– Jimmy Hoffa, Commented Aug 25, 2011 at 18:26

Matt McHugh · Accepted Answer · 2011-08-25 18:22:37Z

2

Since you actually have a business layer, that's where I'd put it. You should let the business layer handle the finding/creating of an organization, so all the WebForms code needs to do is ask for an IpasLogOnManagement.IpasOrganization object based on the name the user selected (lstOrganizations) or entered (txtOrganizationName).

answered Aug 25, 2011 at 18:22

Matt McHugh

2561 silver badge4 bronze badges

Add a comment |

Jimmy Hoffa · Accepted Answer · 2011-08-25 18:24:09Z

2

Your trusty coder's nose led you correctly to question this, Separation of Concerns states you should have these in different modules, and I couldn't agree more. The UI should worry about presenting the UI, and passing along the user activities to the appropriate other modules. When SoC is utilized, your separate modules can be used by more than one UI (or non-UI modules).

answered Aug 25, 2011 at 18:24

Jimmy Hoffa

1,56510 silver badges19 bronze badges

Add a comment |

Ivan · Accepted Answer · 2011-08-26 14:54:30Z

A part of this code could be moved to the business logic.
I would rewrite this method like this:
Manager (BLL):

    public IpasLogOnManagement.IpasOrganization FindOrganizationByName(string name)
    {
        IpasLogOnManagement.IpasOrganization result = null;
        try
            {
                result = organizationProvider.SearchForOrganizationByName(name);
            }
            catch (IndexOutOfRangeException)
            {
                result = null;
            }
        return result;
    }

UI (WebForm):

   private string GetSelectedOrganizationName()
   {
       string result = txtEmail.Text;
       if (OrganizationsSeparateFromUsers)
       {
           if (lstOrganizations.SelectedIndex == -1)
           {
               result = txtOrganizationName.Text;
           }
           else
           {
               result = lstOrganizations.Items[lstOrganizations.SelectedIndex].Text;
           }
       }
       return result;
   }

   //Instead of 'EnsureOrganizationExists'
   private IpasLogOnManagement.IpasOrganization GetSelectedOrganization()
   {
       var organizationName = GetSelectedOrganizationName();
       var organization = manager.FindOrganizationByName(organizationName);
       if (organization == null)
       {
           organization = manager.CreateOrganization(organizationName);
       }
       return organization;
   }

A couple of notes:
- I’ve changed the name of the method from EnsureOrganizationExists to GetSelectedOrganization.
- I would consider another way to check if the organization exists. It’s not right to catch an exception of a specific type to do this.

Stack Exchange Network

Should this code be in the code behind or separate class?

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Should this code be in the code behind or separate class?

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions