I have two simple classes. One is called UseCounter<T>
and is a simple container for other objects while holding some information about its use (how many times it's been already used and if it can be reused once again).
Then there is Dispenser<T>
, which holds a collection of UseConter<T>
and is used to get the right element from the collection and then update its information.
The trouble I have is in the Dispenser's Get()
method. It should return the object with the lowest combination of Count
and TotalCount
, and then call Increase()
method in order to increase count.
However, when I run this code, Dispenser<T>
always returns the same element. It's like the Linq'd element wasn't a reference to the object, but its copy, so the Increase()
method increases only the local object's properties, but not the one in the collection.
I am at loss here, because I have never encountered such a behaviour.
Here comes the code:
UseCounter:
public class UseCounter<T>
{
public T Element { get; private set; }
public int TotalCount { get; private set; }
public int Count { get; private set; }
public bool Useful { get; set; }
public UseCounter(T element)
{
this.Element = element;
this.Count = 0;
this.TotalCount = 0;
this.Useful = true;
}
public void IncreaseCounter()
{
this.Count++;
this.TotalCount++;
}
public void DecreaseCounter()
{
if(this.Count == 0)
{
throw new ArgumentOutOfRangeException("Count cannot be lower than 0!");
}
this.Count--;
}
}
Dispenser:
private readonly object _elementLocker = new object();
private IEnumerable<UseCounter<T>> _elements;
public IEnumerable<T> Elements
{
get { return _elements.Select(e => e.Element); }
}
public int FinishedCount
{
get
{
lock (_elementLocker)
{
return _elements.Where(e => e.Useful == false).Count();
}
}
}
public int CurrentlyWorkingCount
{
get
{
lock (_elementLocker)
{
return _elements.Where(e => e.Count > 0).Count();
}
}
}
public Dispenser(IEnumerable<T> elements)
{
this._elements = elements
.Distinct()
.Select(e => new UseCounter<T>(e));
}
public T Get()
{
lock(_elementLocker)
{
var lCount = _elements
.Where(e => e.Useful == true)
.Select(e => e.Count).Min();
var lTCount = _elements
.Where(e => e.Useful == true)
.Where(e => e.Count == lCount)
.Select(e => e.TotalCount).Min();
var el = _elements
.Where(e => e.Useful == true)
.First(e => e.Count == lCount && e.TotalCount == lTCount);
el.IncreaseCounter();
return el.Element;
}
}
public void Report(T element, bool successful)
{
lock(_elementLocker)
{
var el = _elements
.First(e => e.Element == element);
el.DecreaseCounter();
if(el.Useful == true && successful == true)
{
el.Useful = false;
}
}
}
This is the problem:
this._elements = elements
.Distinct()
.Select(e => new UseCounter<T>(e));
This is lazy. Every time you iterate over _elements
, it will go back to the original source data, find a distinct set, and then project that set onto a new sequence of UseCounter
instances. Any changes you make to those instances are irrelevant to what happens when you next iterate over _elements
.
If you're happy for the set of input elements to be "frozen" on construction, you can just materialize the query so it only executes once:
this._elements = elements
.Distinct()
.Select(e => new UseCounter<T>(e))
.ToList();
Now every time you iterate over elements
, you're iterating over the same sequence of references.
See more on this question at Stackoverflow