2
\$\begingroup\$

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<Session>(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<Session>(id);

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

        return result;
    }
}

I am using it in this way:

public class Chapter {
//....

    public static Chapter[] GetChildren(int parentId)
    {
        string sqlQuery = baseSqlQuery + Environment.NewLine +
                 "WHERE CHAPTERS.GroupId=? AND CHAPTERS.ParentId=?";                

        return DSDatabase.RunQuery<Chapter>(sqlQuery, SessionManager.Current.GroupId, parentId).ToArray();
    }

//....
}

It seems to me as not a very good solution, because my code becomes tightly coupled and hides the dependencies. I'm looking now for better solutions to this issue.

\$\endgroup\$
1
\$\begingroup\$

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.

\$\endgroup\$
0
\$\begingroup\$

Intensive use of mutable static state always fights back. I would split it to three projects:

School.dll

    public class Chapter
    {
        // ...
    }

    public class Session
    {
        public int GroupId { get; set; }
        // ...
    }

    public interface ISessionReader
    {
        Session Read(int id); 
    }

    public interface ISessionWriter
    {
        void Write(Session session);
    }

    public interface IChapterReader
    {
        Chapter[] Read(int groupId, int parentId);
    }

School.Data.dll (references School.dll)

    public class SessionReader : ISessionReader
    {
        // ...
    }

    public class SessionWriter : ISessionWriter
    {
        // ...
    }

    public class ChapterReader : IChapterReader
    {
        // ...
    }

    internal class DSDatabase
    {
        // ...
    }
}

School.UI.dll (references School.dll, School.Data.dll)

    public class SessionViewModel
    {
        public SessionViewModel(
            ISessionReader sessionReader, 
            ISessionWriter sessionWriter, 
            IChapterReader chapterReader)
        {
            SessionReader = sessionReader;
            SessionWriter = sessionWriter;
            ChapterReader = chapterReader;
        }

        public void Load(int sessionId) => Session = SessionReader.Read(sessionId);
        public void Save() => SessionWriter.Write(Session);
        public Chapter[] FindChildren(int parentId) =>
            ChapterReader.Read(Session.GroupId, parentId);

        Session Session { get; set; }
        ISessionReader SessionReader { get; }
        ISessionWriter SessionWriter { get; }
        IChapterReader ChapterReader { get; }
    }

Hope it helps to grasp the idea. Please let me know if you would like me to be more specific. Exceptions are omitted for brevity.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.