search

RISCSoftware / cpacs_tigl_gen

Generates CPACS schema based classes for TiGL
Apache License 2.0
5 stars 5 forks source link

Check whether one option is valid in choices #8

Closed rainman110 closed 7 years ago

rainman110 commented 7 years ago

It would be great, of the generator could handle choices in a better way.

Currently, in case of the choice A or B, both A and B are optional. The generated code does not check, that one of both options is in fact valid.

bernhardmgruber commented 7 years ago

I have thought about this issue multiple times and there is also a branch "choices" where I have done some prototyping. The thing is that such a kind of validation, and recognition which choice is present in the CPACS file respectively, is harder than I thought. Or at least I did not come up with a good and easy solution yet. Anyway, this issue probably takes some time and will eventually be implemented. However, I cannot give you an estimate when this happens.

bernhardmgruber commented 7 years ago

I guess I have a solution. The generator now creates an additional member function

TIGL_EXPORT bool ValidateChoices() const;

for all classes which have at least one choice. It validates if the currently set members (optionals and vectors) match the choices specified in the CPACS schema. This function is now automatically called at the end of ReadCPACS() and if it fails, an error is logged.

This solution is implemented on the https://github.com/RlanderRISCSW/cpacs_tigl_gen/tree/choices branch and I generated a new TiGL with it at https://github.com/RlanderRISCSW/tigl/tree/cpacs_3_choice_validation. I have tested it by removing an etaStart in a ribsPositioning and it logged the error. Feel free to test it as well.

rainman110 commented 7 years ago

The code to the choice validation looks good. I personally would make the method private as it is only called by ReadCPACS.

A public method you could add is a generic validate function, which returns true or false. This could call validateChoices and other potential validation routines.

bernhardmgruber commented 7 years ago

This is correct from your point of view ;) However, we internally provide a GUI for editing some of the CPACS elements containing choices and would like to give the user feedback in case the current set of values does violate the schema. So we need check whether the choices are valid after each edit. I therefore would like to keep the method public.

Regarding a generic validate() function, yes we could provide one. However, I think it is always better if a class is defined in a way which requires no validation (e.g. using the type system to force correctness). This might also be possible for implementing the choice validation by e.g. generating types for each subelement in a choice and stuffing those types in a variant. Unfortunately, this is quite difficult to implement^^ So me might settle on a validate() method for all classes which have at least one validateXXX() submethod (I plan validation of minOccurs and maxOccurs, which may become an additinal validateXXX() method).

bernhardmgruber commented 7 years ago

I merged the choices branch into master.