3
\$\begingroup\$

I have so many classes like this in my solution but I've chosen this one just to show you guys what I mean by it.

In my solution I have lots of classes that just follow the same 2 classes exactly the same, except for the class name and for different purposes.

The issue is, I think I'm OP'ing it. Does something this simple, something I do with around 50 class pairs in my solution really need to take up 2 files and this much code?

It seems like it can be improved but I just can't work out a well-driven way, someone said something about structs but I didn't really look into it too much and it's been too long to remember what else they said.

Is there some sort of layout or structure I can do to reduce the amount of code or the time it takes to code this? I feel like somethings staring me right in the face and I'm missing it.

The purpose of this class pair is to cache database information, store it in a dictionary and return it in case you need it later. It saves an extra call to the database every time I need to get something, and it's just a lot easier to maintain and work with than constant database calls all over the place.

internal class SubscriptionHandler
{
    private readonly Dictionary<int, Subscription> _subscriptions;

    public SubscriptionHandler()
    {
        _subscriptions = new Dictionary<int, Subscription>();
    }

    public void Load()
    {
        using (var dbConnection = Program.Server.DatabaseHandler.Connection)
        {
            dbConnection.SetQuery("SELECT * FROM `subscriptions`;");

            using (var reader = dbConnection.ExecuteReader())
            {
                while (reader.Read())
                {
                    var subscriptionId = reader.GetInt32("id");
                    _subscriptions[subscriptionId] = new Subscription();
                }
            }
        }
    }

    public bool TryGetSubscriptionById(int subscriptionId, out Subscription subscription)
    {
        return _subscriptions.TryGetValue(subscriptionId, out subscription);
    }
}

internal class Subscription
{
    public int Id;
    public string Name;
    public string Badge;
    public int Credits;
    public int Pixels;
    public int Respects;

    public Subscription(int id, string name, string badge, int credits, int pixels, int respects)
    {
        Id = id;
        Name = name;
        Badge = badge;
        Credits = credits;
        Pixels = pixels;
        Respects = respects;
    }
}
\$\endgroup\$
  • \$\begingroup\$ (Welcome to CR!) Whatever the pros and cons of coding Subscription & SubscriptionHandler thus: are you getting at an "intra-pair-of-classes" problem, or do you see one between all those (pairs of?) classes you have lots of? If it is between different pairs, present at least one more. \$\endgroup\$ – greybeard Dec 3 '17 at 7:48
  • \$\begingroup\$ This doesn't look like caching; this looks like loading the whole table and keeping it in memory while it is needed. Is this exactly the real code? new Subscription() shouldn't compile; I would guess there is meant to be type specific loading code there. Is that the case? \$\endgroup\$ – VisualMelon Dec 3 '17 at 9:45
  • 1
    \$\begingroup\$ This approach could be problematic..........what if there's an update to the database - will your 'cached' database manage that? What are the consequences of failure: will a plane fall out of the sky or is the consequence negligible? There is nothing wrong with constant database calls - in fact it eliminates 'temporary variables' (like what is being done here) --- unless performance is a huge concern. Is it a concern? \$\endgroup\$ – BKSpurgeon Dec 3 '17 at 22:23
2
\$\begingroup\$

So you're saying that you have a bunch of classes representing db rows, and need a WhateverHandler for each class?

Use generics.

https://msdn.microsoft.com/en-us/library/ms172192(v=vs.110).aspx https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/generics/

The type is replaced whenever you need to use it, like so:

internal class Handler<T> where T : new()
{
    private readonly Dictionary<int, T> _rows;
    private readonly string _tableName;
    private Handler() {}
    public Handler(string tableName)
    {
        _rows = new Dictionary<int, T>();
        _tableName = tableName;
    }

    public void Load()
    {
        using (var dbConnection = Program.Server.DatabaseHandler.Connection)
        {
            dbConnection.SetQuery("SELECT * FROM `{0}`;".Format(tablename));

            using (var reader = dbConnection.ExecuteReader())
            {
                while (reader.Read())
                {
                    var rowId = reader.GetInt32("id");
                    _rows[rowId] = new T();
                }
            }
        }
    }

    public bool TryGetById(int rowId, out T row)
    {
        return _rows.TryGetValue(rowId, out row);
    }
}

edit: thanks @kuskmen for reminding me about the new() constraint

\$\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.