Skip to content

Conversation

@sharadkjaiswal
Copy link
Contributor

MaximumItem and MinimumItem don't handle mixed array of integer and
double values properly.

MaximumItem, MinimumItem and Sort methods don't replicate.

  • Updated the input argument type as IEnumerable so that the argument is marshaled as 1D collection, and hence it will replicate on multi-dimension collection.
  • On multi dimension collection these methods fail or behave wrong. It should replicate and return values from each collection.
  • Fixed http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-3567

MaximumItem and MinimumItem don't handle mixed array of integer and
double values properly.
Fixed by using DoubleConverter as item selector for Min() and Max
methods.
Fixes http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-3992
@sharadkjaiswal
Copy link
Contributor Author

@Steell, @ke-yu

@ke-yu
Copy link
Contributor

ke-yu commented Jul 25, 2014

Does List.MinimumItem(list:[]): var[]..[] work for all kinds of object that implements IComparer? If it does, we can't assume it always returns double...

@ke-yu
Copy link
Contributor

ke-yu commented Jul 25, 2014

Otherwise LGTM

- Updated the input argument type as IEnumerable<object> so that the
argument is marshaled as 1D collection, and hence it will replicate on
multi-dimension collection.
- On multi dimension collection these methods fail or behave wrong. It
should replicate and return values from each collection.
- Fixed http://adsk-oss.myjetbrains.com/youtrack/issue/MAGN-3567
@sharadkjaiswal
Copy link
Contributor Author

Are we making any assumption about double? It works on all objects of same type that implement IComparer, but integer and double are interchangeable in Dynamo and hence we should be able to cast integers to double, else we leave rest of the objects as it is.

…3992

Conflicts:
	test/DynamoCoreTests/Nodes/ListTests.cs
ke-yu added a commit that referenced this pull request Jul 29, 2014
@ke-yu ke-yu merged commit 35bdabe into DynamoDS:master Jul 29, 2014
@sharadkjaiswal sharadkjaiswal deleted the MAGN-3992 branch July 29, 2014 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants