 |
|
 |
You really need to study up on what a singleton is. This is not it. This is nothing more than a session variable wrapper, and one with some serious issues as well.
|
|
|
|
 |
|
 |
Why all this hassle and introduce potential thread safety issues when all you want to solve is this?:
public static class Application
{
public static User CurrentUser
{
return Membership.GetUser(HttpContext.Current.User.Identity.Name);
}
}
You could even cache it if you want so that it doesnt constantly call the database, but that's pretty much it
|
|
|
|
 |
|
 |
i agree with you but whatif we need some extra properties like firstname lastname or preferences for users. like i have mentioned above.
|
|
|
|
 |
|
 |
Then you can create your own Membership provider and configure it in the web.config. I have done it in several projects. Works quite well
|
|
|
|
 |
|
|
 |
|
 |
Small remark: Your "singleton per user" is not ThreadSafe - it is not the case, but still..
|
|
|
|
 |
|
 |
isn't private static IUser objUser = new UserClass();
makes it thread safe?
|
|
|
|
 |
|
 |
No, not that part
get
{
if (null == Session[IUserSessionName])
{
if (User.Identity.IsAuthenticated)
{
string userID = Membership.GetUser().ProviderUserKey.ToString();
objUser = new UserClass().GetUserInformation(userID); Session[IUserSessionName] = objUser;
}
}
else
{
objUser = (IUser)Current.Session[IUserSessionName];
}
return objUser; }
See my comments: 1 and 2.
Scenario:
- 1st user login and call property 'get'
- //1 line execute in his thread
- 2nd user login and call property 'get'
- //1 line execute in his thread (2nd user that is)
- First user execute line //2 - he returns 2nd user info
|
|
|
|
 |
|
 |
One thing certainly wrong in my case is
that statement if (HttpContext.Current.User.Identity.IsAuthenticated)
has to come before this
if (null == System.Web.HttpContext.Current.Session[userProfileSesssionNmae])
{
which means it should be
if (HttpContext.Current.User.Identity.IsAuthenticated)
{
if (null == System.Web.HttpContext.Current.Session[userProfileSesssionNmae])
{
string userID = Membership.GetUser().ProviderUserKey.ToString();
objUserProfile = new AccountBLL().GetUserProfile(userID);
System.Web.HttpContext.Current.Session[userProfileSesssionNmae] = objUserProfile;
}
}
else
{
objUserProfile = (IUser)System.Web.HttpContext.Current.Session[userProfileSesssionNmae];
}
Because when user is athuenticated then it will be unique session .i will update my this topic with this one. thanks for pointing it .
|
|
|
|
 |
|
 |
Well, I still think that your code is incorrect.
The entire objUser static field is useless. Why do you need that field? .. It is shared between users - my scenario is still valid (in newest version).
If you remove the static field from your code - you will solve the problem.
.. or Am I missing something?
|
|
|
|
 |
|
 |
its very useful for individual users because i have mentioned this clearly above that its apin to get always Membership.getuser().Provider etc to get userid and then to get their email address but what if you want some hands on information like theeir firstname , lastname or their any information which needs to be accessable easily then that registry class is very useful . And ofcourse need to be static
" IUser is a static property which can be used on any page from Singleton.IUserInstance.UserID." You can either add namespace to basepage or to web.config and accessible everywhere
|
|
|
|
 |
|
 |
the IUser objUser variable should be a local variable declared inside the get property... this will remove the threading issue.
this is what focussed programmers do
|
|
|
|
 |
|
 |
i am going to test code with multi threads first
|
|
|
|
 |
|
 |
Like many other threading and synchronization problems, testing might never uncover this!
This is how the problem will show itself:
1. The thread processing user1's request might be switched out just before returning the IUser object.
2. Then the thread processing user2's request overwrites the shared/static variable with user2's IUser object.
3. When user1's request thread resumes, it will return the IUser object for user2
This will never happen if the IUser object is not shared between users, such as simply making it local.
this is what focussed programmers do
|
|
|
|
 |
|
 |
i have used now private static readonly object padlock = new object();
padlock to make the above code thread safe
|
|
|
|
 |
|
 |
Or...you could have just made objUser local to the property and then not have to lock at all. There is absolutely no need for objUser to be static.
|
|
|
|
 |
|
 |
This is simply a wrapper class around Session[]. Is being confused with a Singleton
|
|
|
|
 |
|
 |
Kindly do some research on Singleton Design Pattern. I would like to know where we can use it a pure ideal Singleton Design Pattern in asp.net. I have explained it before its per user Singleton Design Pattern. Again it fullfills the concept and syntax for singleton. It gives you ease of using userintance at global level
|
|
|
|
 |
|
 |
There is no such thing as a "per user Singleton Design Pattern" (which is actually a contradictory statement). This is the "store a static variable in a Session object" pattern. You could've just as easily put the object in an Application object for global access, then you could do away with the static variables and locks. Either way, this is NOT the Singleton Pattern. Sorry, but this solution is crap.
|
|
|
|
 |
|
 |
dont be a Master of a line
|
|
|
|
 |
|
 |
You should also mark your singleton as readonline
public static readonly object MySingleton = new Object();
-Steven
|
|
|
|
 |
|
 |
yes. You are right in ideal Singleton class. but for my example i cannot make public static IUser IUserInstance readonly, because imagine on user preferences page if user change its Firstname or Surname then all i have to do is Singleton.IUserInstance.firstName ='AnyName' and it will update the value in session as well. pretty easy though.
|
|
|
|
 |
|
 |
Declaring it readonly will only mark the variable containing the object as being read-only, not the individual properties of the object. This will prevent accidental overwriting or recreation of the singleton. For a singleton pattern making the singleton object a read-only one is recommended. A public set accessor to the property is also not needed, best would be to remove it, or at least declare it as private.
|
|
|
|
 |
|
 |
public static IUser IUserInstance if i declare it private then it will not be accessible globally on all pages and also declaring this property readonly will not allow user preferences page to set the value .
|
|
|
|
 |