topcoder-greed icon indicating copy to clipboard operation
topcoder-greed copied to clipboard

More concise way to render param types and values

Open wookayin opened this issue 11 years ago • 9 comments

The old way

For example, we have templates such as

${<if !e.Output.Param.Type.Array}
        __expected = ${e.Output}
${<else}
        __expected = (${foreach e.Output.ValueList v ,}
            ${v}${end}
        )
${<end}

or

${<if !in.Param.Type.Array}
            ${in.Param.Type.Primitive} ${in.Param.Name} = ${in};
${<else}
${<if !Options.cpp11}
            ${in.Param.Type.Primitive} ${in.Param.Name}[] = {${foreach in.ValueList v ,}
                ${v}${end}
            };
${<else}
            vector<${in.Param.Type.Primitive}> ${in.Param.Name} = {${foreach in.ValueList v ,}
                ${v}${end}
            };
${<end}
${<end}

Suggestion

A little messy, isn't it? Instead, I imagine a way that the parameter value is rendered directly in a polymorphic way: if it was a scalar, render it as-is, and if was a vector, render it in the way the compiler can interpret it.

We can know the language trait informations and the way how the parameter values should be rendered, given the context and the options.

The new way

Here are examples:

        __expected = ${e.Output}
  • Assuming the parameter is a primitive -- rendered as 1 or "YES" or 3.14159265358979 ...
  • Assuming the parameter is an array. Currently, ${e.Output} renders into the form of {1, 2, 3, 4, 5}. The modified render emits (1, 2, 3, 4, 5) -- the tuple, which is the way it should be written in python.

or

            ${in.Param.Type} ${in.Param.Name} = ${in};
  • Type becomes, for example, int, vector<int>, vector<long long> (in C++), or int[] (in Java), and so on...
  • ${in} (or in.Value ?) becomes 3, {1, 2, 3}, ... (no foreach blahblah)
    • Here comes a pitfall. Without C++11, we can't use initializer lists. But given the rendering context, we are provided whether C++11 feature is enabled or not in the configuration. So an alternative is required depending on this.
    • Surely, if we can't achieve it in a generic way, then we should branch the cases (as right now)
    • Or, introducing ${in.Declaration} -- writing the variable declaration code in the Java code, not in the template?

Remarks

  • This is just an idea for improvement - not a major nor pressing issue.
  • Considering various ways and cases, w.r.t various languages and parameter types (and grid-like options for String[] maybe), there are lots of things to be concerned.
  • Hope it helps us to maintain a much clean and readable template.

wookayin avatar Feb 08 '14 16:02 wookayin

I'm almost done with this feature (coming soon!) This is slightly related to #142.

wookayin avatar Mar 20 '14 16:03 wookayin

I just found out a fundamental design error, which causes this issue (messy template) and also related to #142 and #151. That is, the ParamValue class, should not try to cover both array values and normal values. The right design should be two separate classes, ParamValue and ParamValueList, in which ParamValueList contains a list of ParamValue.

In this case, the language renderer should be able to render ParamValueList separately and get the correct format. User can choose between render the ParamValueList directly or foreach over its values and render each ParamValue.

Right now the code is wrong when try to convert to values from TC API model to greed model. In AbstractLangauge.java line 64, the renderParamValue method is called. However, no rendering should happen at this time. The data model should be language independent and the rendering is delayed to the code generation phase.

@vexorian @wookayin What do you guys think?

zen0wu avatar May 06 '14 14:05 zen0wu

@shivawu I totally agree with you. Actually, I had caught the deficit and was making a way to fix this. Unfortunately I was too busy these days so I didn't complete the job. If you wish, I will share the working experimental branch which is in a temporary status.

As I previously mentioned in the other issues, the ParamValue (indeed it is Argument conceptually) should contain the value in the canonical form (i.e. independent of language). I think we might introduce a breaking API changes (2.0 is still in a tentative status!), which is expectedly to be accompanied by a little changes on templates (e.g. deprecating the usage of *.ValueList). I hope we could discuss on it after sharing the works.

wookayin avatar May 06 '14 15:05 wookayin

Yes, please share your working branch and I'll take a look, then we can find the best way to do this.

zen0wu avatar May 07 '14 02:05 zen0wu

What do you think of this commit?

It basically separate the ParamValue and ParamValueList, and the templates are still the same. The new data template engine solves the problem of #142 and #151. But it's not fully tested, and simplifying the template is going to be a tough work.

This is only a proposal, if you have better solutions, that would be great.

zen0wu avatar May 07 '14 08:05 zen0wu

@shivawu Hello, we were maybe too busy these days. I finally managed to continue my previous refactoring and improvements on this issues: it's almost done. I am now sharing my branch, please feel free to look at it.

Technical details on the implementations you might be interested in can be found in the commit messages.

wookayin avatar Jul 06 '14 13:07 wookayin

@wookayin I've gone through your change of param value briefly, got a few questions. So the argument is a container of type and value, in which the type can be a primitive or an array, and the ParamValue is changed to a Iterable<Argument>, but what does the ParamValue stand for exactly?

Also, when I implement my branch (param-value), I found that when we start a rendering process, the language related engine is fixed, so when rendering the data template, there's no way for the engine to know that here we must use a bare engine instead of the language specific engine. So your way to solve this problem, is to distinguish the argument (which is a pure value), and the param value (which is language related), and use argument in the data template rather than the param value to render it to its canonical form, it this right?

zen0wu avatar Jul 22 '14 14:07 zen0wu

I am sorry I missed your previous comment. Here are my answers: as of current branch (up to commit cca51df9),

  • Argument: The actual value passed to the method(function), each bound to a single parameter.
  • ParamValue: The value object itself, consisting the type and the value representation in a string (canonized)

Actually the names have to be exchanged (Argument <-> ParamValue), to fit to the our well-known definition. Some names (especially, ParamValue) are inappropriate.

I had already noticed this, but it seems to be introducing a backward-incompatibility on templates; so we might need some discussions. I remember that I once wrote a commit of exchanging those class names, but I see I didn't attached the commit into this branch. It looks that you agree with this, don't you?

wookayin avatar Sep 09 '14 03:09 wookayin

And yes, to distinguish the argument (currently ParamValue, but Argument afterwards) and the value (current Arguemnt -- wrong, but what appropriate name it will have?) themselves.

The value's are rendered into appropriate forms (e.g. 1LL in C++, 1L in Java) according to the way that the LanguageRenderer specifies.

wookayin avatar Sep 09 '14 03:09 wookayin