4
\$\begingroup\$

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;
    }
\$\endgroup\$
1
  • \$\begingroup\$ +1 Perfect question for the site, which will help inform future developers about where boundaries between modules should be. \$\endgroup\$ Aug 25 '11 at 18:26
2
\$\begingroup\$

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).

\$\endgroup\$
2
\$\begingroup\$

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).

\$\endgroup\$
0
\$\begingroup\$

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.

\$\endgroup\$

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.