Return Object with different scope based on Conditional Logic (C#) -


i've been developer (also in professional capacity) little while , never focused on clean / structured code. self taught guess i'm missing of fundamentals. reading books never fills gaps. hope great experience post.

so point, have method returns object (campaign) based on conditional logic.

if can object via "campaignviewmode" must have been "viewed" therefore else last inserted

  • 1, if has been viewed (cookie)
  • 2, else last inserted.

pretty basic code has serious "code smell" (repetition). in ideal world i'd remove conditional.

  public campaign getdefaultcampaign()     {         campaign campaign = null;          using (userrepository userrepo = new userrepository())         {                var user = userrepo.getloggedinuser();              if (user != null)             {                 string campaignviewmode = "";                 if (httpcontext.current.request.cookies["campaignviewmode"] != null)                 {                     campaignviewmode = httpcontext.current.request.cookies["campaignviewmode"].value.tostring();                 }                  //get last worked on/viewed                 campaign = _context.tbl_campaign                     .where(x => x.name == campaignviewmode & x.tbl_usertocampaign                                                                 .where(z => z.userid == user.userid & z.campaignid == x.campaignid)                                                                 .select(u => u.userid)                                                                 .firstordefault() == user.userid)                     .select(y => new campaign()                     {                         campaignid = y.campaignid,                         name = y.name,                         webname = y.webname,                         dateadded = y.dateadded                     }).firstordefault();                  //or last inserted                 if (campaign == null)                 {                     campaign = _context.tbl_campaign                     .where(x => x.name == campaignviewmode & x.tbl_usertocampaign                                                                 .where(z => z.userid == user.userid & z.campaignid == x.campaignid)                                                                 .select(u => u.userid)                                                                 .orderbydescending(d => d.dateadded).firstordefault() == user.userid)                     .select(y => new campaign()                     {                         campaignid = y.campaignid,                         name = y.name,                         webname = y.webname,                         dateadded = y.dateadded                     }).firstordefault();                 }             }         }         return campaign;     } 

could kindly point me in right direction of removing conditional or @ last reduce "smell" ?

fully appreciate time!

regards,

there's alot going on here. here's i'd do.

  1. don't new instances (as repository). code against abstracts (irepository) provided di container injected class constructor.

  2. remove duplication maps data model returned model (select(x=> new campaign()). extract out method or separate responsibility entirely.

  3. remove huge nested if(user!=null). check right front , return if null.

  4. refactor 2 fetching operations behind interface (igetcampaigns) , create 2 classes; 1 fetches latest inserted, , 1 fetches last viewed/worked on. inject 1 other form decorator chain, wired di container.

probably lot take in if you're unfamiliar these concepts; happy go through offline if you'd like.


Comments