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

Load from dictionary or database if not found

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

Problem

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;
    }
}

Solution

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.

Code Snippets

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

Context

StackExchange Code Review Q#154168, answer score: 6

Revisions (0)

No revisions yet.