2
\$\begingroup\$

I think here is something wrong with code. I use a class with methods to get tenants from DB:

    public List<CrmTenant> GetAllTenants()
    {
        List<CrmTenant> tenantsList = new List<CrmTenant>();

        try
        {
            var crmTenants =
                 from tenant in context.CrmTenant
                 select tenant;

            if (crmTenants != null)
            {
                foreach (var tenant in crmTenants)
                {
                    tenantsList.Add(tenant);
                }
            }

            return tenantsList;
        }
        catch (Exception ex)
        {
            this.logger.Write("Failed to get all tenants from database.", "Exceptions", TraceEventType.Error, ex);
            return null;
        }
    }

    /// <summary>
    /// Gets Tenant by name.
    /// </summary>
    /// <returns></returns>
    public CrmTenant GetTenantByName(string tenantName)
    {
        CrmTenant crmTenant = null;

        try
        {
            crmTenant =
                 (from customerCrm in context.CrmTenant
                  where customerCrm.TenantName == tenantName
                  select customerCrm).Single();
            return crmTenant;
        }
        catch (Exception ex)
        {
            this.logger.Write(String.Format("Failed to get tenant '{0}' from database.", tenantName), "Exceptions", TraceEventType.Error, ex);
            return null;
        }
    }

    /// <summary>
    /// Gets tenant by Username.
    /// </summary>
    /// <returns></returns>
    public CrmTenant GetTenantByUserName(string userName)
    {
        CrmTenant crmTenant = null;

        try
        {
            crmTenant =
                 (from customerCrm in context.CrmTenant
                  where customerCrm.Username == userName
                  select customerCrm).Single();
            return crmTenant;
        }
        catch (Exception ex)
        {
            this.logger.Write(String.Format("Failed to get tenant by username '{0}' from database.", userName), "Exceptions", TraceEventType.Error, ex);
            return null;
        }
    }

Here is call for methods

// get tenant
var crmTenant = CrmServiceHandlerFactory.GetInstance().Resolve<TenantManager>().GetTenantByUserName(userName);

I am new in programming, could you please help me to improve this code

\$\endgroup\$
2
  • \$\begingroup\$ Why do you think there is something wrong with your code? \$\endgroup\$ Commented Jul 25, 2012 at 9:11
  • \$\begingroup\$ Because I have ten methods like GetTenantByName GetTenantByUserName GetTenantBySize GetTenantByIce and so on... I doubt that it's a good style... Or i'm wrong? \$\endgroup\$ Commented Jul 27, 2012 at 10:22

1 Answer 1

3
\$\begingroup\$

I can see some room for improvement:

  1. Most importantly, you shouldn't hide exceptions like this. You should most likely let the exceptions bubble up and let the called decide what to do when an exception happens. And if you return null, it means you will either have to add null checks everywhere, or you will get NullReferenceException instead of the exception that actually caused the problem, which makes it harder to debug.
  2. You don't need to add items into the list one by one, you can do it all at once using AddRange(). Or even better, just use ToList():

    return crmTenants.ToList();
    
  3. This one is really minor, but the query from tenant in context.CrmTenant select tenant can be simplified to just context.CrmTenant, the LINQ doesn't do anything useful there.

#1 is by far the biggest issue, I think, because it shows an error in your design. #2 and #3 could just simplify your code a bit.

\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.