10

I recently came across the below piece of code in our application

var updateDefinition = new UpdateDefinitionBuilder<OverviewProfile>()
            .Set(a => a.Advisors[-1].IsCurrent, advisor.IsCurrent);

In the above code, Advisors is a List and UpdateDefinitionBuilder is from MongoDB driver.

Could you please let me know the use of -1 in the index of the list?

Editing after the below comments/answers

The OverviewProfile class is as below:

public class OverviewProfile: BaseInvestorProfile
{
    //Other properties

    public List<Advisor.View.Advisor> Advisors { get; set; }
    public OverviewProfile(int id): base(id)
    {
        Advisors = new List<Advisor.View.Advisor>();
    }
}

And this is this the working code. This code updates the data to the mongo db based on the condition. There are no other methods inside this class, just other properties.

This is one class, but the same usage is there for properties of multiple class, and even when we add a new List property and check, it works fine.

gvk
  • 597
  • 7
  • 27
  • This shouldn´t work at all. Usually an index of -1 is returned by methods like `InfexOf`, to return the first occurence of a particular instance. -1 usually means this item wasn´t found. Apart from this an index of -1 passed to a `List` will cause an `ArgumentException`, as the index must not be smaller zero. – MakePeaceGreatAgain Dec 20 '17 at 09:38
  • 2
    Is it possible that the implementer or Advisors has overridden the square bracket indexer? I'd be *guessing* that they're using is to apply the value to all elements in the list. But that's really nasty – LordWilmore Dec 20 '17 at 09:39
  • I would assume that `.Advisors` is *not* an instance of the .NET `List`. Its a special implementation that has overloaded the `[index]` operator, like LordWilmore said, and that `-1` addresses the last entry in the list. – Michael Dec 20 '17 at 09:42
  • 1
    What does `a.Advisors[-1]` return, when you debug it. I can hardly imagine it even *does*. – MakePeaceGreatAgain Dec 20 '17 at 09:53
  • I think Evk is right, more information: https://stackoverflow.com/questions/11367902/negative-list-index – Nash Carp Dec 20 '17 at 09:54
  • 2
    Is this `List` really `System.Collections.Generic.List`? – Ahmad Khan Dec 20 '17 at 09:58
  • 2
    I´m starting to think we should close that question as unclear, as from the currently posted code it´s impossible to run that code without an exception. There must be something hidden, that we can only guess about as long as we have no [Minimal, Complete, and Verifiable example](https://stackoverflow.com/help/mcve) – MakePeaceGreatAgain Dec 20 '17 at 10:03
  • @HimBromBeere OP updated the post with the code. It is broken indeed. – Patrick Hofman Dec 20 '17 at 10:06
  • This is a list @MuhammadAhmad, am not able to debug that line, it just skips past it – gvk Dec 20 '17 at 10:07
  • @PatrickHofman I allready saw, but from that it´s impossible to run successfully. – MakePeaceGreatAgain Dec 20 '17 at 10:07
  • Indeed, unless that code never got executed... @HimBromBeere – Patrick Hofman Dec 20 '17 at 10:07
  • @gvk did you step *into* (F11) with your debugger when you´re at the `Set`-line? – MakePeaceGreatAgain Dec 20 '17 at 10:07
  • Am not getting any exception or there are no other implementations/overload for this. @HimBromBeere, yes with F11 – gvk Dec 20 '17 at 10:07

2 Answers2

14

You are using overload of UpdateDefinitionBuilder<T>.Set which expects Expression. This expression is not compiled and executed directly but instead is converted to native mongodb syntax and is used as part of mongo db query (same as how Entity Framework or other ORM converts expressions to SQL). This basically says "update all overview profiles and set IsCurrent flag of first advisor that matches criteria to advisor.IsCurrent value". Because mongodb allows negative indexes (meaning - relative to the end of collection) - C# mongodb driver can convert your expression to valid mongodb query (but see update below).

Update. As stated here, -1 still has special meaning for mongodb C# driver. It will be converted to positional update operator $:

the positional $ operator acts as a placeholder for the first element that matches the query document

For example:

var updateDefinition = new UpdateDefinitionBuilder<OverviewProfile>()
    .Set(a => a.Advisors[-1].IsCurrent, false);

colleciton.UpdateOne(c => c.Advisors.Any(r => r.IsCurrent), updateDefinition);

Will be converted to something like:

"updates": [{
        "q": {
            "Advisors": {
                "$elemMatch": {
                    "IsCurrent": true
                }
            }
        },
        "u": {
            "$set": {
                "Advisors.$.IsCurrent": false // <- your expression
            }
        }
    }
]

However, point about negative index meaning relative to the end of colleciton in mongodb still holds, because any other negative index except -1 (for example, -2), will be converted to query like this:

{ "$set" : { "Advisors.-2.IsCurrent" : false} }
Evk
  • 98,527
  • 8
  • 141
  • 191
1

If a.Advisors is a List<T> indeed, there is no use. That will throw an ArgumentOutOfRangeException.

What is more likely, that this is some custom class or implementation over List<T> where the -1 value passed onto the indexer means something like the 'default' value.

Whatever it is, I think it is a bad design and should be avoided.

Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325
  • Just to add an official reference... Look at the [source code](http://referencesource.microsoft.com/mscorlib/R/0c65bec4d3fac21e.html) – Camilo Terevinto Dec 20 '17 at 09:40
  • "implementation over `List`"? The indexer of a list isn´t virtual, you can´t override it (you can hide it however). – MakePeaceGreatAgain Dec 20 '17 at 09:50
  • The *class* can have a custom implementation, the indexer indeed can be hided using `new`. @HimBromBeere – Patrick Hofman Dec 20 '17 at 09:51
  • @CamiloTerevinto I can't believe I'd not thought of that unchecked (uint) trick. I'm not sure if it's horrifying or just mildly amusing to see it in the BCL. – VisualMelon Dec 20 '17 at 12:24