Getting value from session if available, if not, take from the database

Question

This method is a constructor that has to decide which URL to add to the response header as Allow-Origin. i am actually adding the Allow-Origin later according to the value of PermittedOrigion variable. I am using Session to eliminate database access.

Could i write it more efficient? I am working with asp.net webform

public ResponseAsJson(HttpRequest request, int affiliateId)
    {
        string cacheKeyName = $"PermittedOrigion_{affiliateId.ToString()}";
        const string notDefined = "N";
        object ValueFromCache = HttpContext.Current.Cache[cacheKeyName];

        if (!String.IsNullOrWhiteSpace(ValueFromCache?.ToString()))
        {
            if (ValueFromCache.ToString() == notDefined)
                return;

            PermittedOrigion = ValueFromCache.ToString();
            return;
        }

        string actualOrigion = null;

        if (request.UrlReferrer != null)
           actualOrigion = request.UrlReferrer.GetLeftPart(UriPartial.Authority);

        string[] strFromDB = RM.BL.Affiliate.Instance(affiliateId).LUAffiliate.AccessControlAllowOriginJsonResult.Split(';');

        if (!String.IsNullOrWhiteSpace(actualOrigion) && strFromDB.Contains(actualOrigion))
        {
            PermittedOrigion = actualOrigion;
        }
        else 
        {
            string strLocalhost = strFromDB.Where(x => x.ToLower().StartsWith("http://localhost")).FirstOrDefault();

            if (!String.IsNullOrWhiteSpace(strLocalhost))
                PermittedOrigion = strLocalhost;
        }

        if (PermittedOrigion == null)
        {
            HttpContext.Current.Cache[cacheKeyName] = notDefined;
        }
        else
        {
            HttpContext.Current.Cache[cacheKeyName] = PermittedOrigion;
        }           


    }

Could you add a couple of tags about the technologies you used? Like asp.net or asp.net-core, entity-framework etc? — t3chb0t
– t3chb0t, Commented Apr 8, 2018 at 8:57

momo · Accepted Answer · 2018-05-02 13:39:34Z

Some recommendations:

1) Give your method a better name. ResponseAsJson is not what it does. Proposal: AppendAllowOriginHeader

2) Your method relies on the affiliateId inputparameter. I would change the parameter to int? affiliateId and then add a short guard:

if(!affiliateId.HasValue) {
   throw new ArgumentNullException(nameof(affiliateId)); 
}

Also keep in mind, that the id may be invalid in which I would prefere to throw an InvalidArgumentException

3) Let's clean up: Extract the method in two subfunctions:TryAddFromCache(string key) and TryAddFromAllowedOrigins

TryAddFromCache could look like:

if(String.IsNullOrWhiteSpace(key)) {
   return false;
}   

...
return true;

And in AppendAllowOriginHeader:

if (TryAddFromCache()) {
    return;
}

4) As a rule: Use const or var wherever possible.

5) Extract a second method:

bool TryAddFromAllowedOrigins(IEnumerable<string> allowedOrigins) { ... }

6) Performance: In this case we have the database access as bottleneck. The party starts at:

RM.BL.Affiliate.Instance(affiliateId).LUAffiliate.AccessControlAllowOriginJsonResult.Split(';');

.. which has the most impact.

Your array strFromDB will probably not hold references to several thousand strings. So filtering or sorting (which should be the work of the db anyway) has no impact.

 string strLocalhost = strFromDB.Where(x => x.ToLower().StartsWith("http://localhost")).FirstOrDefault();

can be changed to:

strFromDB.FirstOrDefault(x => x.ToLower().StartsWith("..."));

7) Consider using better names: for example strFromDB could be changed to allowedOrigins

Stack Exchange Network

Getting value from session if available, if not, take from the database

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Getting value from session if available, if not, take from the database

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions