Skip to content

Conversation

@pboyer
Copy link
Contributor

@pboyer pboyer commented Sep 20, 2017

Purpose

This PR removes the ability to index arrays by anything but an integer. This is the first step in an separatio between array and dictionary behaviors in DesignScript's built-in types.

Currently, the language implements associative arrays quite similarly to Lua's Tables:

https://www.lua.org/pil/2.5.html

This branch makes the language more similar to two of the most popular scripting languages: Python and JavaScript. Both of these languages make a clear delineation between these two types.

FYIs

@ke-yu @Racel @aparajit-pratap

<packages>
<package id="Greg" version="1.0.6176.18754" targetFramework="net45" />
<package id="Newtonsoft.Json" version="8.0.3" targetFramework="net45" />
<package id="NUnit" version="3.8.1" targetFramework="net45" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this package added to Dynamo core?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, weird.

@Racel
Copy link
Contributor

Racel commented Sep 20, 2017

Sounds good to me

@kronz
Copy link
Contributor

kronz commented Sep 20, 2017

Cool, so this is simply to remove the entrypoint to dictionaries (not their existence), right?

@pboyer
Copy link
Contributor Author

pboyer commented Sep 20, 2017

@kronz @Racel This is being pursued as an experiment at present. I'd really appreciate if you can read up on lua's tables, because it might explain some of the rationale for associative arrays in DS: https://www.lua.org/pil/2.5.html

@Racel
Copy link
Contributor

Racel commented Sep 21, 2017

image
print(a["x"]) --> 20 s very confusing to me. Just because b=a, why would a=b?
and then a=nil --now only 'b' still refers to table is also strange

image
I love the last line "You can introduce subtle bugs in your program if you do not pay attention to this point" :)

I understand the rationale now, but even after reading this, I believe that separating dictionaries is the right thing to do which will make for clearer UX for the user and better UI strategy for us in the future

@kronz
Copy link
Contributor

kronz commented Sep 25, 2017

So, tables in Lua seem pretty cool, and there defintely seems like a common rationale. I'm with Racel, that a cleaner UX around list v dictionary behavior through seperation still seems like a better approach. "Simple, Coherent, Capable". Tables in Lua certainly seem Capable, but not simple.

BTW, just for fun, if you put some of the same code in identified in the link into a CBN, you get some pretty nutty results:
a = {};
k = "x";
a[k] = 10 ;
a[20] = "great";
(a["x"]);
k = 20;
(a[k]);
a["x"] = a["x"] + 1;
(a["x"]);

@pboyer pboyer changed the title (Experimental) Separate associative arrays into Dictionary, List types Separate associative arrays into Dictionary, List types Dec 8, 2017
Copy link
Contributor Author

@pboyer pboyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aparajit-pratap I can't approve of this PR because I opened it, but I do approve. I have a few minor comments.

I have some broader comments, but I'll try and address them with a PR for you instead once this is merged.

class ArrayMarshaler : PrimitiveMarshler
{
private FFIObjectMarshler primitiveMarshaler;
private readonly CLRObjectMarshler primitiveMarshaler;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a follow up PR, we should definitely fix these spelling errors. :D

</data>
<data name="FailedToConvertArrayToDictionary" xml:space="preserve">
<value>Cannot convert List to Dictionary type.</value>
</data>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's replace Array with List in InvalidArrayIndexType and FailedToConvertArrayToDictionary

@@ -0,0 +1,59 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine. I think it's clear that one contains builtin DesignScript code and the other is builtin code contained in an assembly.

@aparajit-pratap aparajit-pratap merged commit 6493a83 into DynamoDS:master Dec 21, 2017
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.

6 participants