Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate Dictionary Items in a strongly-typed fashion #11

Open
hfloyd opened this issue May 11, 2022 · 5 comments
Open

Generate Dictionary Items in a strongly-typed fashion #11

hfloyd opened this issue May 11, 2022 · 5 comments

Comments

@hfloyd
Copy link
Contributor

hfloyd commented May 11, 2022

As per @NikRimington from umbraco/Umbraco-CMS#11854 (comment):

I would love it if Dictionary items could generate a single "DictionaryAliasses" class automatically that followed the hirearchy in the dictionary section to make accessing the aliases safer.

For example:

public static class DictionaryAliases
{
     public const String TopLevelDictionaryItemWithNoChildren = "Top Level Dictionary Item With No Children";
    
     public static class TopLevelDictionaryItemWithChildren
     {
             public const string Alias = "Top Level Dictionary Item With Children";
             public const string FirstChildNode = "First Child Node";
     }
}

I hope that makes some sort of sense. Then dictionary items can be a) Compile time safe, and b) runtime exception caught. They can be accessed like this: DictionaryAliases.TopLevelDictionaryItemWithChildren.Alias

@abjerner
Copy link
Member

Hi @hfloyd

I haven't really looked into this yet due to lack of time.

In your example, TopLevelDictionaryItemWithChildren doesn't really have a value of it's own. Du you have any examples for when it does? The alias of a dictionary item may also contain characters that aren't good for C# member names.

When using nested dictionary items, I tend to use the parent alias as a prefix for the child item's alias. Eg. Something like:

  • MyWebsite probably empty
    • MyWebsite.Footer probably empty
      • MyWebsite.Footer.Address ➡️ Address
        • MyWebsite.Footer.Address.Street ➡️ Street
        • MyWebsite.Footer.Address.City ➡️ City

People may create dictionary items in lots of different ways, so it might be difficult coming up with something that can handle this.

@hfloyd
Copy link
Contributor Author

hfloyd commented Nov 12, 2022

Hi @abjerner ! This isn't my example - It's Nik's (@NikRimington) I just copied it over as a courtesy.

@hfloyd
Copy link
Contributor Author

hfloyd commented Nov 12, 2022

I agree with you that If I have nested Dictionary items, the "folders" are usually blank. Of course, really any dictionary item could return a blank value, since it isn't required via the UI.

In terms of the aliases not being good for C# names... I've used a little string extension (.MakeCodeSafe()) when I've wanted to dynamically generate thinks like HTML ID attributes, etc. So I don't think that would be the biggest challenge to overcome.

@abjerner
Copy link
Member

Both my own Skybrud.Essentials (which is already a dependency) and Umbraco also have some logic for creating safe strings. But stripping out unwanted characters could potentially cause conflicts if two different aliases are converted to the same safe string. But if that happens, simple solution could be to return a Models Builder says no style error.

Another issue I hadn't though of before is to actually resolve the dictionary values. If our Models Builder generates one or more static classes with constants (or static readonly fields/properties), we can't really resolve the values in a DI-friendly way, and as a result, the code wouldn't be very testable. That might be bad in some cases, but fine in others.

But if the constants are mapped to the aliases instead of the values, then we shouldn't have problem, but it would require developers to resolve the values on their own.

@hfloyd
Copy link
Contributor Author

hfloyd commented Nov 15, 2022

But if the constants are mapped to the aliases instead of the values, then we shouldn't have problem, but it would require developers to resolve the values on their own.

I think that would actually be ideal - so that MB doesn't need to manage the actual Dictionary lookup code - since that is part of CMS.Core and might change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants