HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Load/Save application sessions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
sessionssaveapplicationload

Problem

I'm working on load/save application sessions functionality, and came up with this code:

```
public class SessionManager
{
private static ISessionStorage storage = new DbTableSessionStorage();
private static StudySession session;
public static StudySession Current
{
get
{
if (session == null) {
throw new Exception("Load or start new session before using SessionManager.Current");
}

return session;
}
}

public static void LoadSession(int sessionId)
{
session = storage.LoadSession(sessionId);
}

public static void Save(StudySession session, int prefferedId = -1)
{
storage.SaveSession(session, prefferedId);
}
}

public class DbTableSessionStorage : ISessionStorage
{
public void SaveSession(StudySession studySession, int prefferedId = -1)
{
Session session = DSDatabase.Context["USER"].Read(prefferedId);
if (session == null) {
session = new Session();
}

if (prefferedId != -1) {
session.Id = prefferedId;
}

session.Name = studySession.Name;
session.Timestamp = studySession.Timestamp == DateTime.MinValue
? DateTime.Now
: studySession.Timestamp;

session.GroupId = studySession.GroupId;
session.TestId = studySession.TestId;
session.Position = studySession.Position;
session.SoundOn = studySession.SoundOn;

DSDatabase.Context["USER"].Save(session);
}

public StudySession LoadSession(int id)
{
StudySession result = new StudySession();

Session session = DSDatabase.Context["USER"].Read(id);

if (session != null) {
result.Id = session.Id.Value;
result.Name = session.Name;
result.Timestamp = session.Timestamp;
result.GroupId = session.GroupId;
result

Solution

Current property

You should never throw the Exception. There is almost always a more appropriate exception already available or you should create a new one. The name of the exception usually already indicates the reason for it.

In this case I would pick the InvalidOperationExctption.

LoadSession method

Is the user allowed to re-load the session? If not then either throw an exception if he tries it again or return the already loaded session with the coalesce operator:

session = session ?? storage.LoadSession(sessionId);


I would also prefer if the method returned the loaded session too so I can immediately use it without switching to the property:

return (session = storage.LoadSession(sessionId));


You can assign it and return it in a single line.

StudySession vs Session classes

I woudn't create another session object. I think you should just reuse the Seesion you get from the database. It already has everything you need. You are just copying the values.

Your code would become much simpler without this mappings.

Chapter class

To loosen the coupling you would need to make the methods of this classes non-static and pass to the constructor an instance of the DSDatabase or its interface and maybe a SessionManager but I don't entirely understand what it is good for. Actually you need a group id for the GetChildren query. So maybe pass it as a parameter.

Code Snippets

session = session ?? storage.LoadSession(sessionId);
return (session = storage.LoadSession(sessionId));

Context

StackExchange Code Review Q#140103, answer score: 2

Revisions (0)

No revisions yet.