Update src/FluentNHibernate/MappingModel/Collections/LayeredColumns.cs #156

Open
wants to merge 1 commit into
from

Projects

None yet

4 participants

@CV1
CV1 commented Aug 15, 2012

This class uses a Hashset intern. This is no problem with .Net, but mono (at least version 2.10.8 and sooner) has a incorrect Hashset implementation, so this class is not workin correct with the mono runtime. As long the hashset in mono is not fixed, I suggest to use a Dictionary here.

Using a Dictionary in this class instead

@CV1 CV1 Update src/FluentNHibernate/MappingModel/Collections/LayeredColumns.cs
This class uses a Hashset intern. This is no problem with .Net, but mono (at least version 2.10.8 and sooner) has a incorrect Hashset implementation, so this class is not workin correct with the mono runtime. As long the hashset in mono is not fixed, I suggest to use a Dictionary here.

Using a Dictionary in this class insteas
8e13db8
@jagregory
Owner

@CV1 I'm happy to merge these changes, but as we don't have a Mono CI build running I have no way to know if they actually work (only that they don't break the .Net side).

@jagregory
Owner

I can't merge this. There's a spec failure, when_subclass_map_has_a_has_many_to_another_entity.should_only_use_one_column_in_the_target_entity_s_key (see: SubclassSpecs.cs#L54).

Looks like there's something relying on the uniqueness provided by the Hashet.

@wsky
wsky commented on 8e13db8 Sep 6, 2012

i also found it...

codesharp/infrastructure#18

@ivaylo5ev ivaylo5ev commented on the diff Oct 15, 2015
...NHibernate/MappingModel/Collections/LayeredColumns.cs
@@ -17,9 +17,8 @@ public IEnumerable<ColumnMapping> Columns
yield break;
var maxLayer = layeredValues.Keys.Max();
- var values = ((HashSet<ColumnMapping>)layeredValues[maxLayer]);
-
- foreach (var value in values)
+ var values = ((Dictionary<int,ColumnMapping>)layeredValues[maxLayer]);
@ivaylo5ev
ivaylo5ev Oct 15, 2015

In reference to below comment, the usage would become:
var values = ((Dictionary<ColumnMapping, ColumnMapping>) layeredValues[maxLayer]).Keys
foreach (var value in values) ...

@ivaylo5ev ivaylo5ev commented on the diff Oct 15, 2015
...NHibernate/MappingModel/Collections/LayeredColumns.cs
if (!layeredValues.ContainsKey(layer))
- layeredValues[layer] = new HashSet<ColumnMapping>(new ColumnMappingComparer());
-
- ((HashSet<ColumnMapping>)layeredValues[layer]).Add(mapping);
+ layeredValues[layer] = new Dictionary<int, ColumnMapping>();
@ivaylo5ev
ivaylo5ev Oct 15, 2015

I might suggest a closer to the hash set implementation like this:
layeredValues[layer] = new Dictionary<ColumnMapping, ColumnMapping>();
((Dictionary<ColumnMapping, ColumnMapping>) layeredValues[layer]).Add(mapping, mapping);

The idea is to use the dictionary.Keys collection, as it is essentialy effective as a hash set. I do not know however, if Mono does work well with dictionaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment