Skip to content

Add support for input parameter array initialization#28

Merged
ognjenkatic merged 17 commits into
masterfrom
feat/master/array-initalization
Jul 29, 2022
Merged

Add support for input parameter array initialization#28
ognjenkatic merged 17 commits into
masterfrom
feat/master/array-initalization

Conversation

@boma96

@boma96 boma96 commented Jul 29, 2022

Copy link
Copy Markdown
Collaborator

Now it is possible to specify array as input parameter

builder.AddTask(
                wf => wf.ArrayTask,
                wf =>
                    new()
                    {
                        Integers = new[] { 1, 2, 3 },
                        Models = new[]
                        {
                            new ArrayTaskInput.TestModel { String = "Test1" },
                            new ArrayTaskInput.TestModel { String = "Test2" }
                        },
                        Objects = new dynamic[] { new { AnonymousObjProp = "Prop" }, new { Test = "Prop" } }
                    }
            );

@boma96 boma96 requested a review from ognjenkatic July 29, 2022 06:36

@ognjenkatic ognjenkatic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One note regarding type conversions

Comment on lines +76 to +81
private static JArray ParseArrayInitalization(NewArrayExpression newArrayExpression)
{
if (newArrayExpression.NodeType != ExpressionType.NewArrayInit)
throw new Exception("Only dimensionless array initialization is supported");

return new JArray(newArrayExpression.Expressions.Select(ParseExpression));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This seems to return [ 1,2,3] as ["1", "2", "3"]. The reason is

 if (expression is ConstantExpression cex)
                return Convert.ToString(cex.Value);

@boma96
We can extract the correct type, but this might break something somewhere. However, most frameworks seem to be equipped to handle string to int conversion automatically if it is really convertible. I think we should do this, what do you think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It will also break our tests

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for pointing this out, did not notice it

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ok, will make separate PR for this

@boma96 boma96 requested a review from ognjenkatic July 29, 2022 10:55
@ognjenkatic ognjenkatic merged commit e7f3baf into master Jul 29, 2022
@ognjenkatic ognjenkatic deleted the feat/master/array-initalization branch July 29, 2022 11:05
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