NamingConvention and IIndexable or IDictionary

[petrhaus]

Hi Tim,
it seems we have a major bug... if we pass in an IIndexable or IDictionary, DotLiquid does not take into account NamingConvention and keys resolution fail.
I've been looking on a way to resolve but found anything, do you have any idea?

[petrhaus]

uhm seems I got it resolved, but have to check twice...

UPDATE:
the same happens with Anonymous Objects too...

[tgjones]

Hi Alessandro,

This is an interesting question! I'm pretty sure that (rightly or wrongly) this behaviour was intentional on my part. The motivation behind naming conventions was obviously to allow C# developers to use the naming conventions they are used to when writing their drops and filter methods, but let template authors use standard Liquid / Ruby naming conventions.

But that doesn't apply to IIndexable or IDictionary, whose keys are (presumably) strings, and not code identifiers.

Anonymous objects are different again, and perhaps should have the naming convention applied. I hesitate to make that kind of very-backwards-incompatible change - what do you think? Are you finding it's a real issue?

[petrhaus]

Hi Tim,
sorry my the late reply.
It's not a real issue per se, but sometimes it leads to confusion. I think it's not worth changing it at this time, but it would be nice if we can plan it for a major version. We could just use a double checking mechanism, so we won't introduce a backward-incompatible change...

[earlNameless]

To me the behavior should be consistent everywhere. If the naming convention does not apply everywhere, I cannot tell people "the template is case insensitive", because that is partial truth. It is case insensitive when you use code identifiers, but otherwise it does not (there are other exceptions, see below). This seems like a leaky abstraction.

Template writers should not care whether something is an IDictionary, an anonymous class, or a Drop. To them it is just a container with properties they can access by name.

It might seem difficult to implement due to the following cases:

1. IDictionary with keys that differ only by case
2. A drop with properties whose names differ only by case (already happens?)

But to me, if someone picked a naming convention that is case insensitive, it is their responsibility to not let this happen or to know this is not possible given their Hashes/Drops/etc. LiquidMarkup processing should just pick the first thing it comes across that matches in a case insensitive manner.

I found other places where this applies as well (depending on the result here, they will be created as separate issues):

1. true/false/null/nil do not follow casing of naming convention (I have not reproduced this)
2. Sort does not appear to take naming convention into account when picking which properties to sort on (RespondTo does not take NamingConvention into account)

[tgjones]

I agree, and I do plan to standardise this behaviour in 2.x. I agree with you that if developers choose a case-insensitive naming convention, then it's reasonable to expect all name lookups to be case-insensitive too.

On your secondary points:

1. true / false / null / nil happen to all be lower-case in both C# and Ruby, so is this a real issue?
2. Good point about Sort, I hadn't caught that one.

[earlNameless]

1. That is true; I still consider it an issue: True / False are the keywords in VB.NET which can reference DotLiquid (and null is Nothing, but that is a different discussion).

To me effectively any look ups have to take the naming convention and string comparison into account. I am using this naming convention:

public class CaseInsensitiveNamingConvention : DotLiquid.NamingConventions.INamingConvention {
    public StringComparer StringComparer {
        get { return StringComparer.OrdinalIgnoreCase; }
    }

    public string GetMemberName(string name) {
        return name;
    }
}

Which to me, would mean that anything can be specified in any case (constants, tags, filters, filter arguments, built-in variables, etc.).

It might be that I am trying to use LiquidMarkup for something it was not intended to be used for. If I am going someplace with this that does not fit into the purpose of DotLiquid, please let me know.