Best way to Structure class + Instantiation

Question

Is this a good way to design a class so I can prevent values being changed/ the state must remain as it was when it was intantiated...

This code would be normally instantiated after a call from the database.

public class InstructionWorkflow : IStageWorkflow
{
 private readonly int _companyID;
 public InstructionWorkflow(int instructionid, int progressid, int companyID, InstructionKeyInfoDTO keyInfo, InstructionStageProgress currentStage)
 {
 _companyID = companyID;
 CurrentStage = currentStage;
 KeyInfo = keyInfo;
 ProgressID = progressid;
 InstructionID = instructionid;
 }
 public int InstructionID { get; private set; }
 public int ProgressID { get; private set; }
 public InstructionKeyInfoDTO KeyInfo { get; private set; }
 private InstructionStageProgress CurrentStage { get; set; }
 public bool IsMyCompanyOwnerOfStage
 {
 get { return CurrentStage.StageOwnerID == _companyID; }
 }
 /// <summary>
 /// #1 stage cannot be ticked as completed
 /// #2 stage must be initiated by the user, this will mean stage is open
 /// This is the best way so we can code against opening and closing a stage elsewhere,
 /// that will have a lot of rules on its own
 /// #3 stage must be owned by loggedon users company
 /// </summary>
 public bool IsStageOpenForUpdates
 {
 get { return IsMyCompanyOwnerOfStage && !CurrentStage.IsCompleted && CurrentStage.DateInitiated != null; }
 }
 public int? StageOwnerID
 {
 get { return CurrentStage.StageOwnerID; }
 }
}
public interface IStageWorkflow
{
 bool IsMyCompanyOwnerOfStage { get; }
 bool IsStageOpenForUpdates { get; }
 int? StageOwnerID { get; }
 int ProgressID { get; }
 int InstructionID { get; }
 InstructionKeyInfoDTO KeyInfo { get; }
}

Update: As per suggestions

public class InstructionWorkflow : IStageWorkflow
{
 private readonly IUserSession _userSession;
 private readonly IReadOnlySession _readOnlySession;
 public InstructionWorkflow(int instructionid, int progressid, IUserSession userSession, IReadOnlySession readOnlySession)
 {
 _userSession = userSession;
 _readOnlySession = readOnlySession;
 ProgressID = progressid;
 InstructionID = instructionid;
 CurrentStage = ResolveWorkflow();
 }
 public int InstructionID { get; private set; }
 public int ProgressID { get; private set; }
 public InstructionKeyInfoDTO KeyInfo { get; private set; }
 private InstructionStageProgress CurrentStage { get; set; }
 public bool MyCompanyIsOwnerOfStage
 {
 get { return CurrentStage.StageOwnerID == _userSession.MyProfile.CompanyID; }
 }
 /* NOTES:
 * #1 stage cannot be ticked as completed
 * #2 stage must be initiated by the user, this will mean stage is open
 * This is the best way so we can code against opening and closing a stage elsewhere,
 * that will have a lot of rules on its own
 * #3 stage must be owned by loggedon users company
 */
 public bool StageIsOpenForUpdates
 {
 get { return MyCompanyIsOwnerOfStage && !CurrentStage.IsCompleted && CurrentStage.DateInitiated != null; }
 }
 public int? StageOwnerID
 {
 get { return CurrentStage.StageOwnerID; }
 }
 private InstructionStageProgress ResolveWorkflow()
 {
 var currentStage = _readOnlySession.Single<InstructionStageProgress>(
 x => x.ProgressID == ProgressID && x.InstructionID == InstructionID);
 if (currentStage == null)
 throw new NoAccessException(string.Format("workflow stage progress not found for instruction #{0} and progressid#{1}",
 InstructionID, ProgressID));
 KeyInfo = null; //set here similar to currentstage above...
 return currentStage;
 }
}

Additional Info: Origincal IUserSession

public interface IUserSession
{
 void SetClientStore(string loginID, string identifier);
 void Logout();
 string LoginID { get; set; }
 string CompanyIdentifier { get; set; }
 UserProfile MyProfile { get; }
}

//new user session

public interface IUserSession
{
 void SetClientStore(string loginID, string identifier);
 void Logout();
 string LoginID { get; set; }
 int UserID { get; set; }
 RoleName Role { get; set; }
 string CompanyIdentifier { get; set; }
 int CompanyID { get; set; }
 CompanyType CompanyType { get; set; }
 string CompanyFriendlyName { get; set; }
 PricePlanType PricePlan { get; set; }
}

Answer 1

The way you're implementing a read-only object looks fine, except for CurrentStage being mutable. Are you sure you want it that way? It seems counter-intuitive with the rest of the class. Seeing as it is a private member anyway, why not make it readonly just like _companyID?

Other comments:

• You are not checking for whether currentStage and keyInfo are null when you read them. It may be better to fail early if you detect this (even if you're sure that it will never happen, a Debug.Assert call could be compiled out in release mode and provides valuable feedback).
• The way you're providing KeyInfo to the rest of the world is prone to breaking the Law of Demeter. This may be the least bad solution, but are you sure you shouldn't provide some wrappers around it just like you did for CurrentStage?
• (削除) Does implementing the public members as properties instead of readonly values really win you anything? The latter would make sure you don't accidentally change it from within the class; on the other hand, the former could (maybe?) work better with reflection. (削除ここまで) After looking into this further, a property is indeed preferrable; however, perhaps give it an explicit readonly backing variable and make it get-only?
• The properties IsMyCompanyOwnerOfStage and IsStageOpenForUpdates both begin with a verb; this sounds off for something that acts like a data member, and I would advise MyCompanyIsOwnerOfStage and StageIsOpenForUpdates respectively.
• I think your class would be readable if you grouped member variables and automatic properties together; as things are, you have to jump around to see what's declared how.
• In my experience, having such long summaries of members leads to unreadable mouseover text; take a look at how much you really gain from putting it all there as opposed to in a remarks (or similar) section.

Comments on updated code:

• I like the way things have been split out to ResolveWorkflow, although I would still advise checking userSession and readonlySession for null.
• I am iffy about _userSession.MyProfile.CompanyId in MyCompanyIsOwnerOfStage -- are you sure this is a valid violation of the aforementioned Law of Demeter? Should an IUserSession really not provide a CompanyID field itself?

• Haroon: Thanks for your advise, I have updated my quote to reflect more clearly what I want to do, I have updated with some of your suggestions, could you please comment? Should I check if IUserSession is null + IReadOnlySession?
• Komi Golov: @Haroon: Edited.
• Haroon: I have added a Guard.ArgumentNotNull(userSession, "usersession"); inside my constructor for both user session + readonlysession
• Haroon: Updated to show my IUserSession, I get what you are saying, but I dont understand why I did that... Maybe it was because this class was being used everywhere else, there is no real reason to be honest with you, it is read only anyway... and it should only ever be accessed by an interface...