patterncsharpMinor
Cookie wrapper in MVC4
Viewed 0 times
wrappercookiemvc4
Problem
I'd like to create a cookie wrapper in my application to help support a DRY approach to coding common cookie methods, provide IntelliSense for cookie-type properties (keys), and protect against typo-errors when working with cookies from multiple classes and namespaces.
The best structure for this (that I can think of) is to apply a base class like such:
Then for each cookie-type that inherits the base class define properties for each key that should be stored in the
Any ideas on how I might improve this design, or suggestions to alternative approaches to attaining the goals I stated at the start of this post?
The best structure for this (that I can think of) is to apply a base class like such:
abstract class CookieBase{
protected HttpRequestBase request;
protected HttpCookie cookie;
protected string name;
public HttpCookie Cookie { }
public string Name { get; }
public HttpCookie Get(){ ... }
protected string GetValue(string key){ ... }
public bool IsSet(){ ... }
protected void SetValue(string key, string value){ ... }
}Then for each cookie-type that inherits the base class define properties for each key that should be stored in the
HttpCookie, and define a constructor that accepts the Request object as a parameter and instantiates the Cookie property of the base class.Any ideas on how I might improve this design, or suggestions to alternative approaches to attaining the goals I stated at the start of this post?
Solution
abstract class CookieBase{Might be just a coding style nitpick, but in C# the scope-opening brace goes on the next line, like this:
abstract class CookieBase
{Also (and this is possibly just a personal preference), when an access modifier isn't specified, it's
public by default. This may or may not be what's intended - the intent is always clearer when the access modifiers are explicit:public abstract class CookieBase
{I like that you're using the
Base suffix to denote a base/abstract class - seeing FooBase immediately tells me that the class cannot be instantiated directly.protected HttpRequestBase request;
protected HttpCookie cookie;
protected string name;protected members should follow the same casing convention as public members - PascalCase:protected HttpRequestBase Request;
protected HttpCookie Cookie;
protected string Name;Now, exposing fields is never a recommended approach. You should expose properties, not fields. Hence:
protected HttpRequestBase Request { get; set; }
protected HttpCookie Cookie { get; set; }
protected string Name { get; set; }Now this causes a naming clash with some other
public members:public HttpCookie Cookie { }
public string Name { get; }The solution would be to make the properties
public, with a protected setter (and remove the clashing public members):public HttpRequestBase Request { get; protected set; }
public HttpCookie Cookie { get; protected set; }
public string Name { get; protected set; }Having a method called
Get is basically a code smell all by itself. I'd get rid of it, its functionality seems to be already implemented in the Cookie property:public HttpCookie Get(){ ... }The rest of your code is not quite reviewable, I'd suggest you first implement it before your post it for peer review:
protected string GetValue(string key){ ... }
public bool IsSet(){ ... }
protected void SetValue(string key, string value){ ... }
}Is this a good approach? Not sure.
abstract classes define abstractions, but your abstract class has nothing that actually warrants being abstract - you have no virtual or abstract members, not even a protected constructor, which leads me to believe that your derived classes might look like this:public class SomeCookie : CookieBase
{
}
public class AnotherCookie : CookieBase
{
}...if that's the case, then the base class is probably a bad design and you're probably better off just calling it
Cookie and making it a normal class.If your goal was to define an abstraction, @Malachi's answer is probably accurate - you'd be better off defining these members in some
ICookie interface (which the "normal" Cookie class described above could very well be implementing).Code Snippets
abstract class CookieBase{abstract class CookieBase
{public abstract class CookieBase
{protected HttpRequestBase request;
protected HttpCookie cookie;
protected string name;protected HttpRequestBase Request;
protected HttpCookie Cookie;
protected string Name;Context
StackExchange Code Review Q#30251, answer score: 2
Revisions (0)
No revisions yet.