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

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
1
down vote

favorite
1












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;










share|improve this question

















  • 2




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

















up vote
1
down vote

favorite
1












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;










share|improve this question

















  • 2




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













up vote
1
down vote

favorite
1









up vote
1
down vote

favorite
1






1





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;










share|improve this 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;












share|improve this question












share|improve this question




share|improve this question








edited Apr 8 at 9:35
























asked Apr 8 at 8:51









Omtechguy

1062




1062







  • 2




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













  • 2




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








2




2




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





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











1 Answer
1






active

oldest

votes

















up vote
1
down vote













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






share|improve this answer





















    Your Answer




    StackExchange.ifUsing("editor", function ()
    return StackExchange.using("mathjaxEditing", function ()
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    );
    );
    , "mathjax-editing");

    StackExchange.ifUsing("editor", function ()
    StackExchange.using("externalEditor", function ()
    StackExchange.using("snippets", function ()
    StackExchange.snippets.init();
    );
    );
    , "code-snippets");

    StackExchange.ready(function()
    var channelOptions =
    tags: "".split(" "),
    id: "196"
    ;
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function()
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled)
    StackExchange.using("snippets", function()
    createEditor();
    );

    else
    createEditor();

    );

    function createEditor()
    StackExchange.prepareEditor(
    heartbeatType: 'answer',
    convertImagesToLinks: false,
    noModals: false,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    );



    );








     

    draft saved


    draft discarded


















    StackExchange.ready(
    function ()
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f191520%2fgetting-value-from-session-if-available-if-not-take-from-the-database%23new-answer', 'question_page');

    );

    Post as a guest






























    1 Answer
    1






    active

    oldest

    votes








    1 Answer
    1






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes








    up vote
    1
    down vote













    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






    share|improve this answer

























      up vote
      1
      down vote













      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






      share|improve this answer























        up vote
        1
        down vote










        up vote
        1
        down vote









        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






        share|improve this answer













        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







        share|improve this answer













        share|improve this answer



        share|improve this answer











        answered May 2 at 13:39









        momo

        1112




        1112






















             

            draft saved


            draft discarded


























             


            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f191520%2fgetting-value-from-session-if-available-if-not-take-from-the-database%23new-answer', 'question_page');

            );

            Post as a guest













































































            Popular posts from this blog

            Python Lists

            Aion

            JavaScript Array Iteration Methods