Logic in convert or in separate methods?

[m-mohr]

Originally posted by @ivorbosloper in #138 (comment)

Also, doing year-specific things in a single function (probably in an overridden Converter.convert() instead of __init__, as convert takes the variant) can bundle the business logic instead of spreading this co-related logic out over many diffferent functions. E.g.

class SomeConverter(BaseConverter):
    def convert(self, *args, **kwargs):
         if self.year < 2018:
             self.columns["xyz"] = "something"
             self.column_migrations["xyz"] = lambda x: f"{x} {x}"
             self.missing_schemas["properties"]["xyz"] = "string"
         elif self.year < 2022:
             # bundled logic here
         else:
             # bundled logic here

         super().convert(*args, **kwargs)

vs spreading out the logic:

class SomeConverter(BaseConverter):
    def get_columns(self):
         if self.year < 2018:
             return old columns
         elif self.year < 2022:
             return intermediate columns
         else:
             return new columns

   def get_missing_schemas(self):
         if self.year < 2018:
            return something with old columns
         elif self.year < 2022:
             return something with intermediate columns
         else:
             return something with new columns

   def get_column_migrations(self):
         if self.year < 2018:
            return something with old columns
         elif self.year < 2022:
             return something with intermediate columns
         else:
             return something with new columns

[m-mohr]

Honestly, for me the spreading out looks much more intuitive to write and understand.

Having a property by default that can be overridden is fine if that works in Python. I'm not overly used to these @... assignments.

[ivorbosloper]

Well, both methods work and it probably depends on the local converter complexity what's best.

The relevant question we were addressing is; is there enough flexibility in the current setup where attributes are overridden in subclasses of BaseConverter? The code that's now in BaseConvert.convert() uses these attributes. I designed the class after all the parameters (both functions and data) of the previous convert_() function. Some additional "hook-options" inside the convert() method might be useful, but I would prefer adding those in relevant use cases...

[m-mohr]

I haven't deeply looked into the whole class-based architecture yet, to be explored based on more converters.
The old template was very easy for newcomers to implement, I'd like to preserve that.