umco / umbraco-vorto

1:1 multilingual property editor wrapper for Umbraco
https://our.umbraco.org/projects/backoffice-extensions/vorto
MIT License
44 stars 42 forks source link

[Improvement] Override Umbraco's HasValue() and GetValue() instead of adding new methods HasVortoValue() and GetVortoValue() #35

Closed Korayem closed 8 years ago

Korayem commented 8 years ago

This way, current Razor views and C# code will work respectful of whether the Data Type is Vorto enabled or not.

This is also important for mixing Vorto with other 3rd Party plugins that aren't aware of Vorto.

mattbrailsford commented 8 years ago

I believe this would just result in an ambiguous reference as the runtime wouldn't know which method to use, hence why we have vorto prefixed versions.

Korayem commented 8 years ago

You can always add global reference to required DLL in Views/web.config to avail such library across all views :+1:

mattbrailsford commented 8 years ago

Thanks for the suggestions, but I think I'd prefer to leave it as is as I don't want to cause any confusion with people not knowing if vorto is being used or the default Umbraco GetValue methods. If this is something you want personally then you can always make your own convenience methods to wrap the GetVortoValue methods, however I think from a general perspective, it is much clearer using the explicit GetVortoValue method what the intention is and thus where to look if problems do arise.

jbreuer commented 8 years ago

The reason this is requested is probably related to the Models Builder. See this for more info: https://github.com/zpqrtbnk/Zbu.ModelsBuilder/issues/72#issuecomment-169644502

mattbrailsford commented 8 years ago

I still don't think this would help though as Models Builder would be coded to use the Umbraco implementation so by having the same method name, it wouldn't automatically just use it, Models Builder would have to make an explicit attempt to use it.