Skip to content

Arrays have some glitchs in C# and Ruby#53

Merged
thorncp merged 2 commits intomasterfrom
arrays
Aug 21, 2014
Merged

Arrays have some glitchs in C# and Ruby#53
thorncp merged 2 commits intomasterfrom
arrays

Conversation

@gregoryjscott
Copy link
Copy Markdown
Contributor

This PR currently fixes and breaks things.

  • Changes the JSON constants to be camelCase to expose issues
  • Adds missing .rb tests

Currently Ruby supports iterating an array but not indexing it. I haven't looked into why yet. The test for iterating the JSON object also fails but I'm not convinced it's needed (see #52).

C# can index an array OK. However, it can't iterate an array and then get the fancy property treatment on the item. I think this is because GetEnumerator() just returns the underlying JObject's GetEnumerator() which isn't a Config class with fancy properties.

Python is all good.

Tasks

  • Fix 2 failing .rb tests
  • Fix 1 .cs tests
  • Make the iterating array tests more robust

This will close #41 after the tests are passing.

@ralreegorganon
Copy link
Copy Markdown
Member

Welp, I've run into this on the C# side now in my usage of this project... guess it's time to focus some attention on this PR.

@ralreegorganon
Copy link
Copy Markdown
Member

The issue with Ruby here is that as soon as we return an array value we lose all the Centroid magic--the objects in the array are just standard Ruby hash objects.

I'm not sure what approach we want to take here, since the only things that come to mind involve reaching into the array and changing the hashes into centroid objects or swapping out the arrays for special centroid arrays, but either way I think we'd end up having to traverse and modify the whole tree...and we've already got concerns about mutability as described in #43.

I'm certainly interested in other ideas.

@thorncp thorncp added this to the 1.1 milestone Jul 9, 2014
@jtroe
Copy link
Copy Markdown
Contributor

jtroe commented Jul 15, 2014

Does C# still have these issues with the ExpandoObject implementation?

@ralreegorganon
Copy link
Copy Markdown
Member

C# doesn't have these issues at all right now, with the changes I've implemented as part of other PRs. The only language that still has issues is ruby, as described in #53 (comment) .

@thorncp
Copy link
Copy Markdown
Contributor

thorncp commented Jul 31, 2014

I pushed a simple change that gets the Ruby tests passing, but comes with the caveat of only supporting 1 dimensional arrays. I recall chatting offline with @ralreegorganon about this awhile ago, and I think we agreed that 1 dimension is fine, as a 2D array that needs the shininess seems like an extreme edge case. Anyone have thoughts about this?

@gregoryjscott
Copy link
Copy Markdown
Contributor Author

Sounds good enough to me. Merge it.

@thorncp thorncp merged commit d980951 into master Aug 21, 2014
@thorncp thorncp deleted the arrays branch August 21, 2014 20:41
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.

Ruby test suite is missing iteration tests

4 participants