-1
\$\begingroup\$

I am new to concurrent dictionaries and all their new methods involving "Try". I have coded a simple method below that retrieves roomInformation from a dictionary (roomInformation is a class) and then prints it if it exists as well as returns true.

Have I done this right? is there a better way? please do let me know if a more robust alternative solution exist.

public bool TryGetRoomInformation(int roomId, out RoomInformation roomInformation)
        {
            if (!_roomInformation.ContainsKey(roomId))
            {
                roomInformation = null;
                return false;
            }

            if (_roomInformation.TryGetValue(roomId, out roomInformation))
            {
                return true;
            }

            roomInformation = null;
            return false;
        }

Useage:

RoomInformation roomInfo;
if (!RoomManager.TryGetRoomInformation(1, out roomInfo))
{
    // didnt find anything :(
    return;
}

// use roomInfo, it was found
\$\endgroup\$
  • \$\begingroup\$ This doesn't look right. Can you post the entire code? It's not possible to review this small snippet. \$\endgroup\$ – t3chb0t Oct 26 '16 at 16:49
  • \$\begingroup\$ Have you considered using locks? Or a transactional database? \$\endgroup\$ – CodesInChaos Oct 27 '16 at 9:41
  • \$\begingroup\$ There's nothing specific to concurrent dictionaries here. TryGetValue is part of the IDictionary<TKey, TValue> interface. \$\endgroup\$ – Peter Taylor Nov 25 '16 at 18:39
1
\$\begingroup\$

One thing I noticed, you can eliminate the first if block, that condition is handled later. If the key is invalid TryGetValue will return false and this way you only lookup the key once:

public bool TryGetRoomInformation(int roomId, out RoomInformation roomInformation)
{
    if (_roomInformation.TryGetValue(roomId, out roomInformation))
    {
        return true;
    }

    roomInformation = null;
    return false;
}
| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$

Worked out the best way of doing this is like this...

public bool TryGetRoomInformation(int roomId, out RoomInformation roomInformation)
{
    return _roomInformation.TryGetValue(roomId, out roomInformation);
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ One difference is, you're not specifically setting roomInformation to null \$\endgroup\$ – tinstaafl Oct 26 '16 at 16:17
  • \$\begingroup\$ Its initially set to null because no parameter is actually passed, its just declared, so roomInformation's default = null. \$\endgroup\$ – Liam Oct 26 '16 at 16:20
  • \$\begingroup\$ One caveat to that is, that in the future it would be easy to forget that and actually pass a RoomInformation object that isn't null. Specifically forcing it to null would cover that eventuality. \$\endgroup\$ – tinstaafl Oct 26 '16 at 18:20
  • 1
    \$\begingroup\$ It will not contain the old value, since out parameter must be set by called method. This is enforced by compiler. And as with other TryX methods, TryGetValue sets out parameter to default(T) if operation failed. \$\endgroup\$ – Nikita B Oct 27 '16 at 7:32
  • 2
    \$\begingroup\$ If it is down to one line then might as well inline it. \$\endgroup\$ – paparazzo Oct 28 '16 at 11:56

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.