Load from dictionary or database if not found

Question

Today I have coded a simple function that will get a room model, if its already been loaded in to the dictionary then it will just return it, if its not in the dictionary if will try its best to load it from the database. I just wondered is this the best way I can do ths? or is there a better way...

public bool TryGetModel(string modelName, out RoomModel model)
{
    if (_roomModels.ContainsKey(modelName))
    {
        _roomModels.TryGetValue(modelName, out model);
        return true;
    }

    using (var mysqlConnection = Sahara.GetServer().GetMySql().GetConnection())
    {
        mysqlConnection.SetQuery("SELECT id,door_x,door_y,door_z,door_dir,heightmap,public_items,club_only,poolmap,`wall_height` FROM `room_models` WHERE `custom`");
        var modelRow = mysqlConnection.GetRow();

        if (modelRow == null)
        {
            model = null;
            return false;
        }

        model = new RoomModel(Convert.ToInt32(modelRow["door_x"]), Convert.ToInt32(modelRow["door_y"]),
            Convert.ToDouble(modelRow["door_z"]), Convert.ToInt32(modelRow["door_dir"]),
            Convert.ToString(modelRow["heightmap"]), Convert.ToString(modelRow["public_items"]),
            Convert.ToInt32(modelRow["club_only"]).ToString() == "1", Convert.ToString(modelRow["poolmap"]),
            Convert.ToInt32(modelRow["wall_height"]));

        _roomModels.Add(Convert.ToString(modelRow["id"]), model); // save it for next time!!
        return true;
    }
}

You are not testing for size (which may be OK). But if you have the space then just loading all the _roomModels up front may be a better approach. — paparazzo
– paparazzo, Commented Feb 1, 2017 at 14:24
What is the data type of modelRow? There is a (extension?) method on DataRow called Field<T> that can be used in lieu of all the Convert calls, assuming that the data isn't purely strings. Not sure if that applies here because I don't know your actual data and I don't know if the corresponding mysql data type (if not DataRow) supports this. — pinkfloydx33
– pinkfloydx33, Commented Feb 1, 2017 at 20:16

Heslacher · Accepted Answer · 2017-02-01 13:46:19Z

Using ContainsKey() together with TryGetValue() seems a little bit too much. Just change it like so

if (_roomModels.TryGetValue(modelName, out model))
{
    return true;
}

which is much easier to read and faster for items which are in the dictionary.

If the columns in the database allow null values you should check first with Convert.IsDBNull() so the method doesn't blow in the users face at any of the Convert.ToXX() methods.

The Where clause of the sql query looks like it is not working.
You should at least let the columns have some space to breathe which makes the query easier to read.

BCdotWEB · Accepted Answer · 2017-02-01 14:51:30Z

Is there a reason not to use an ORM? You need 20 lines just to retrieve one RoomModel from the db, whereas with for instance Entity Framework this could be reduced to at most a handful.

How often do you call this method? Because as one comment suggested, you might be better off loading all models in a dictionary in one go instead of making hundreds of db calls.

I get the TryGetModel() logic and how you're mimicking the logic of TryGetValue(), but isn't it a bit much to return a bool and the model? Wouldn't it be easier to simply return a RoomModel and test if you get a null or an actual value?

Malachi · Accepted Answer · 2017-02-01 14:51:22Z

The way that you explain your code is different from what your code actually does...

if its already been loaded in to the dictionary then it will just return it

it doesn't return the RoomModel directly, it returns a boolean value while indirectly returning the RoomModel in the out variable.

I am not seeing the need to return a boolean value, I think I would rather return the RoomModel itself than try to force a method to return a RoomModel and a boolean value.

Your method is violating the SRP(Single Responsibility Principle) in that you are merging 2 actions into one, checking the list for a specific RoomModel and retrieving a RoomModel from the Database.

Stack Exchange Network

Load from dictionary or database if not found

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Load from dictionary or database if not found

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions