search

duncan-r / SHIP

Simple Hydraulic model Interface for Python
MIT License
7 stars 6 forks source link

Use composition for PathHolder in TuflowFile classes #19

Open duncan-r opened 7 years ago

duncan-r commented 7 years ago

I've been considering this for a while and it's been highlighted a little by the updates in #16 .

The fmp classes hold a PathHolder member, rather than inheriting from PathHolder directly. This may be preferable for the TuflowFile classes as well.

I've considered before whether the different types of ATuflowPart could in fact be simplified a lot and given a keyword type or some such. The command and variable/associates/filepath/etc could then be data/class members.

This could be particularly useful when updating commands that are similar in all ways except an argument on the left of the assignment (e.g. Read GIS Zline THICK == ..\somefile.mid where THICK or THIN are example arguments). It may also help with piped commands, which could potentially be a single part with multiple PathHolder members, although I will have to have a think about this.

It would probably remove some of the complication introduced by the MRO in places and be a bit more "pythonic".

JamesRamm commented 7 years ago

Yes, composition over inheritance is always a more flexible choice.

I think the recent update in #2 is heading toward a similar idea for simplifying ATuflowPart and perhaps just having a generic container class with a flag (or even just lookup on the fly - a cached property perhaps) indicating its' command type