Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Can you use variables in their own initialization line (tryGetOrElse in a dictionary)?

Consider the following code:

private Dictionary<RobotSettings, Trader> createTradersFor(IEnumerable<RobotSettings> settings)
{
    var traderSet = new Dictionary<Tuple<IGateway, IBroker>, Trader>();

    return settings.ToDictionary(s => s, s =>
    {
        var key = Tuple.Create(s.gateway, s.broker);

        Trader trader = traderSet.TryGetValue(key, out trader)
            ? trader
            : traderSet[key] = new Trader(s.gateway, s.broker);
        return trader;
    });
}

I'm talking specifically about the initialization of trader variable in a closure, which uses itself in the same line it is being instantiated.

I've been using that pattern of dealing with dictionaries a lot lately, cause I really don't like uninitialized variables :) and would like to know if this is even guaranteed to compile in the future.

like image 264
Grozz Avatar asked Feb 23 '23 12:02

Grozz


2 Answers

Guarantee is a strong word, but it's very, very, very unlikely that it will stop compiling in the future - the language teams strive to maintain backward compatibility and unless there's a huge paradigm shift (i.e., from VB6 to the first VB.NET), this code should continue building fine.

I actually think it's a nice trick, but it took me some time to see that TryGetValue was used in a ternary operation (and other people had the same problem), so you may be saving one line of code here (moving the trader declaration one line up), but there may be an extra maintenance cost to pay by you later (or whoever inherits this code), so maybe you should reconsider splitting the declaration...

Trader trader;
trader = traderSet.TryGetValue(key, out trader) ?
...
like image 32
carlosfigueira Avatar answered May 13 '23 20:05

carlosfigueira


Beside from looking very odd, there is nothing wrong with it - technically.
First, the declaration of trader will be executed, so there exists a Trader object without a value assigned. Second, the TryGetValue part is evaluated and returns either true or false. If it returned true, the now assigned trader will be returned. If it returns false, a new Trader is created and added to the dictionary via an assignment operation. The result of an assignment operation is the value of the object that was assigned to. That is the new trader. Third, the result of the ternary operator will be returned and assigned to trader.

It is unlikely that this will change in the future, because changing the order of evaluation of a statement like this is a very breaking change.

UPDATE:
Because it looks very odd, I would not use it. I would solve this problem by creating an extension method for IDictionary<TKey, TValue> called GetOrAdd.
It could look like so:

public static TValue GetOrAdd<TKey, TValue>(this IDictionary<TKey, TValue> dict,
                                            TKey key, Func<TKey, TValue> creator)
{
    TValue value;
    if(!dict.TryGetValue(key, out value))
    {
        value = creator(key);
        dict.Add(key, value);
    }
    return value;
}

You would call it like this:

var trader = traderSet.GetOrAdd(key, k => new Trader(s.gateway, s.broker));

This is a lot cleaner and it is even shorter than your odd looking approach.

BTW: You could use the ConcurrentDictionary<TKey, TValue> instead. This class already has a method GetOrAdd and has the benefit of being thread safe.

like image 148
Daniel Hilgarth Avatar answered May 13 '23 20:05

Daniel Hilgarth