jwaliszko / ExpressiveAnnotations

Annotation-based conditional validation library.
MIT License
351 stars 123 forks source link

complex types handling #39

Closed jwaliszko closed 9 years ago

jwaliszko commented 9 years ago

That case is directly related to this thread (more context there).

Basic idea: simplify the support of arrays in the following manner:
[RequiredIf(ArrayContains(Item, Values))]
1st option

I was thinking about something like that, although I'm not fully aware of all pros/cons of such a solution yet:

to be delivered by ExpressiveAnnotations:

js:

typeHelper = {
    array: {
        ...
        tryParse: function(value) {
            if (typeHelper.isString(value)) {
                var array = JSON.parse(value);
                if (array instanceof Array) {
                    return array;
                }                    
            }
            return { error: true, msg: 'Given value was not recognized as a valid array.' };
        },
    ...
    tryParse: function(value, type) {
        switch (type) {
            case 'datetime':
                return ...
            ...
            case 'array':
                return typeHelper.array.tryParse(value);

c#:

internal static class Helper
{               
    public static string GetCoarseType(Type type)
    {
        ...
        if (type.IsArray)
            return "array";
to be done by developers:

Then, anyone who would like to use that built-in array type, should provide appropriate DOM field from where it can be extracted (like in any other case), but this time using templates, since there is no out-of-the-box construction in ASP.MVC for that, so create:

/Type/ArrayTemplate.cshtml:

@using Newtonsoft.Json
@model int[] // or any other /Type/
@Html.Hidden("", JsonConvert.SerializeObject(Model))

and put this in the form:

@Html.EditorFor(model => model.Values, "IntArrayTemplate")

then, respective ArrayContains function, e.g. IntArrayContains (not necesairly built-in) would work:

public bool ArrayContains(int? value, int[] array)
{
    return value != null && array.Contains((int)value);
}

ea.addMethod("ArrayContains", function(value, array) {
    return $.inArray(value, array) >= 0;
});

for such a usage cases:

[RequiredIf("IntArrayContains(Item, Values)")]
2nd option

By looking at proposed solutions (in related thread), which I like, I'm just thinking what would be the advantage of this since everything is sent to client in json most times. But still, it there will be cases when no json is used (but xml?), or we just want to handle the parsing by ourselves, maybe it is enough to just expose complex type parsing like it is done for date, so:

api = {
    settings: {
            parseComplex: undefined,
            parseDate: undefined
            ...
typeHelper = {
    ...
    tryParse: function(value, type) {
        switch (type) {
            ...
            case 'datetime':
            ...
            default:
                if (type === 'complex' && typeof api.settings.parseComplex === 'function')
                    return api.settings.parseComplex(value);
                return { error: true };
            }

The disadvantage is that there is only one parseComplex method to override, so each custom case related to eventual multiple complex types should be somehow distinguished there. But still such cases are rather rare.

3rd option

The idea with dynamic typeHelpers is also attractive (taken from related thread):

ea.addTypeHelper("sometype", function (value) {
    // parse in any way you want and return
});
[CustomTypeHelper("sometype")] 
// or maybe [TypeHelper("sometype")] or [ClientTypeHelper("sometype")] etc.
public IEnumerable<int> Values { get; set; }
4th option

We can add it all.

5th option

This is rather not an option, but we can also change nothing - by choosing that way though developers are going to handle such stuff by themselves, in a way shown by my answers at the beginning of related thread.

jwaliszko commented 9 years ago

I've just unlocked default types parsing: https://github.com/JaroslawWaliszko/ExpressiveAnnotations/commit/ee4e63fae6eb26e989b8e3d12a62cc27ef84d3d8

It was actually no more than that (at least for now it is enough I think):

tryParse: function(value, type) {
    switch (type) {
        case 'datetime':
           ...
        default:
            try {
                return JSON.parse(value);
            } catch (ex) {
                return { error: true, msg: 'Given value was not recognized as valid JSON.' };
            }
    }

Thanks to https://github.com/webwizgordon for help.