i want to refactor the below c# class

Hi,

I have a class that I want to optimize if I can find anything it has below two functions to work on. I see that altough

DemandPerm function is repeatedly called in both functions instead i want to move this to constructor or any suggestions to do anything with this class


 public HttpResponseMessage PutCommand(MeetingCommandRequest commandRequest)
        {
            DemandPerm(SmmPermission.SmmAccess);

            try
            {
                //Assert for invalid Model State
                if (!ModelState.IsValid)
                {
                    return HandleExceptionForInvalidModelState(ModelState);
                }

                if (Log.IsDebugEnabled) Log.DebugFormat("PUT command request received for meeting: {0}", commandRequest.Id);
                _updaterService.ExecuteCommand(commandRequest);
            }
            catch (Exception e)
            {
                return HandleException(e);
            }

            return Request.CreateResponse(HttpStatusCode.Accepted);
        }


   [HttpPut]
        public HttpResponseMessage PutNow(string id, UpdateRequest updateRequest)
        {
            DemandPerm(SmmPermission.SmmAccess);
            
            try
            {
                //Assert for invalid Model State
                if (!ModelState.IsValid)
                {
                    return HandleExceptionForInvalidModelState(ModelState);
                }

                if (Log.IsDebugEnabled) Log.DebugFormat("PUT update request received for meeting: {0}", id);
                _updaterService.UpdateMeeting(id, updateRequest);
                
            }
            catch (Exception e)
            {
                return HandleException(e);
            }

            return Request.CreateResponse(HttpStatusCode.Accepted);
        }

Open in new window

nicedoneAsked:
Who is Participating?
 
apeterCommented:
DemandPerm, should go as like "Authorise" attribute. Decorate you method. You can has custom attribute.

for example...
[AuthorizeUser(AccessLevel = "Create")]
    public ActionResult CreateNewInvoice()
    {
        //...

        return View();
    }
Custom Attribute class as follows.

public class AuthorizeUserAttribute : AuthorizeAttribute
{
    // Custom property
    public string AccessLevel { get; set; }

    protected override bool AuthorizeCore(HttpContextBase httpContext)
    {
        var isAuthorized = base.AuthorizeCore(httpContext);
        if (!isAuthorized)
        {                
            return false;
        }

        string privilegeLevels = string.Join("", GetUserRights(httpContext.User.Identity.Name.ToString())); // Call another method to get rights of the user from DB or from you user cache

        if (privilegeLevels.Contains(this.AccessLevel))
        {
            return true;
        }
        else
        {
            return false;
        }            
    }
}


Also, use log4net for logging...if (Log.IsDebugEnabled) Log.DebugFormat("PUT update request received for meeting: {0}", id);

Logging level(Debug,Info...) can be set in config file . So you don't need to check overtime(if ....) while coding
0
 
ste5anSenior DeveloperCommented:
Two cons:

1) DemandPerm must use exceptions to work. It's a really bad idea to throw exceptions in constructors.
2) DemandPerm (and even the Model.IsValid check) are aspects of the specific methods, not the hosting class.

So when you really want to refactor it, then you should take a look at aspect oriented programming (AOP) and thus PostSharp or Spring.net.
0
 
nicedoneAuthor Commented:
Hi ,

@ste5an, thanks for the reference and guidance

@apeter , for logging part Log is actually of ILog type of log4net so the code should be using log4net in that sense.

For your previous comment, i am a newbie and don't really know how i can convert that DemandPerm function which eventually calls demand method of System.Security.Permissions namespace to an attribute as you specified.

could you guide and help me in the sense that how can i step by step convert that function into an attribute as you mentioned?

thanks
0
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.