New Feature Request - Allow base types to be specified

[dconder]

In a fairly large implementation, with over 20 content types, it would be useful if all of the generated classes inherited from a common base type so we can add global members and have methods that have the base type as a parameter which could be used by all of the derived types. Also extension methods would be available. Would this be possible? I haven't looked at the code base, and I'd be willing to take a stab at this one if it is approved.

[petrsvihlik]

Hi @dconder , it's actually by design that the generated classes don't inherit from a common base type. We wanted to keep the models POCO.

What I would recommend in your case is extending the models using partial classes.

So in case of the Dancing Goat site it could look like this:

CommonBaseType.cs

public class CommonBaseType
{
... shared members
}

CommonBaseTypeExtender.cs

public partial class Article : CommonBaseType {}
public partial class Brewer : CommonBaseType {}
public partial class Cafe : CommonBaseType {}
public partial class Brewer : CommonBaseType {}

Would that work for you?

[dconder]

Hi Petr,

Yep, totally understand you can inherit using the partial. I was just thinking that it would be nice to have an optional base class that only appears if you tell it to. I just checked and we're at 28 content types. It would be nice to be able to just generate and have everything happen with one click. If this isn't something you'd like to add, no worries. I've already written the code and tests for fun, so I can just run with my fork.

[petrsvihlik]

It's definitely a valid requirement. I'm just trying to better understand the context. I need to check with my team and see whether there wouldn't be any unexpected consequences (e.g in terms of strongly-typed deserialization, etc.). We might also need to adjust the best practices for working with models (or at least say what's the preferred approach), etc. We'll get back to you soon.

@JanLenoch @Simply007 @alesk-kentico @TomHruby @sutrkiller @huluvu21 what are your thoughts guys?

[JanLenoch]

Hi @dconder , in your particular case, did you just make the generator to add the : CommonBaseType to the declaration of the model class? Or, did you also make it generate that base class? If so, were its members populated with KC data? Were the shared members modeled using content snippets (in KC)? Thanks!

[dconder]

Hi @JanLenoch I added a new parameter to the command line "--baseclass xxxxx". If this was set, the code generater adds " : xxxxx" to the output of each generated POCO. I also added tests to make sure it works and only happens when --baseclass is set.

[Simply007]

Hi @dconder, is it possible to share the logic you have in the base class?

I agree with @petrsvihlik to have the generated classes as POCO, but I might be missing something when I don't see actual base class implementation.

I believe that common logic for all base classes could be implemented differently.

I am concerned about one thing: When hiding some members in generated classes that are present in the base class. There is no way without the base class code to determine whether to use a new keyword for member declaration.

public class MyBaseClass
{
    public const string Codename = "baseclassCodename";
}

public class KCModel : MyBaseClass 
{
    public (new) const string Codename = "kc_model";
}

This might not be your case, but it could happend.

If we omit new keyword all the times, it is fine for compiling, but it is not a good practice to omit that. In the SDK Type provider, this should not lead to some problems, but I do not see that as a clean implementation. -> Partial class approach does not allow to define member twice.

[dconder]

Hi @Simply007, the base class mainly just contains helper methods. For example, we have some complex logic for determining an items URL, or which image should be rendered at given places. We could use Extension methods and keep the base class empty as well. So it wouldn't have any overrides to the existing generated classes. I

[petrsvihlik]

After giving it a thought we decided to stick with the customization model outlined above.

That means we don't want to include base class types (or any other unnecessary directives) directly in the model classes. All customization should be done via partial classes.

If we are about to have the --baseclass xxxxx parameter implemented in this project, it should be done in the way presented in my first comment - CommonBaseTypeExtender.cs.

Please let us know if you're still interested in implementing it, @dconder.

[dconder]

@petrsvihlik Sure, I'd be happy to help implement it. Let me know if you'd like me to proceed.

[petrsvihlik]

@dconder absolutely! If you accept the invitation that I've just sent you, I'll be able to assign the issue to you. (So that nobody can steal it from you :))

[dconder]

Cool thanks. For tests, how deep would you like them to go? Check that the base class exists, or actually check that it contains the expected code? Should I also create a test that checks that the base class does NOT exist if the parameter to create it is not set?

[petrsvihlik]

I would check whether the class contains expected code (compare it against a fixture, similarly to other tests). Checking the "negative" scenario is nice to have.

[petrsvihlik]

fixed in #70

thank you for the contribution @dconder !