RISCSoftware / cpacs_tigl_gen

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

Implemented check for mandatory empty elements and attributes #9

Closed rainman110 closed 7 years ago

rainman110 commented 7 years ago

Fixes #7

bernhardmgruber commented 7 years ago

As stated in #7, I would like to throw an exception in case of an empty string element. I have just tested this internally for missing mandatory elements: apart from the 2 externalObjects tests, all tests run successful with this change. Therefore I would like to adopt this behaviour for empty string checks as well.

bernhardmgruber commented 7 years ago

Regarding optional elements, we have two options now:

                        if (f.cardinality == Cardinality::Optional && f.typeName == "std::string") {
                            cpp << "if (" << f.fieldName() << " && " << f.fieldName() << "->empty()) {";
                            {
                                Scope s(cpp);
                                cpp << "throw CTiglError(\"Optional " << (isAtt ? "attribute " : "element ") << f.cpacsName << " is empty at xpath \" + xpath);";
                            }
                            cpp << "}";
                        }

or

                        if (f.cardinality == Cardinality::Optional && f.typeName == "std::string") {
                            cpp << "if (" << f.fieldName() << " && " << f.fieldName() << "->empty()) {";
                            {
                                Scope s(cpp);
                                cpp << f.fieldName() << " = boost::none;";
                            }
                            cpp << "}";
                        }

Version 1 enforces clearer intent in CPACS files, version 2 is more tolerant. I slightly prefer version 1, but I am also fine with version 2. What do you think?

rainman110 commented 7 years ago

Version 1 is cleaner, sure.

But it is not consistent with the other checks, where we just write a Log message if an error occured. (See e.g. https://github.com/DLR-SC/tigl/blob/cpacs_3/src/generated/CPACSComponentSegment.cpp#L60).

I think we should decide, whether we want to abort, if the file is not schema conforming, or whether we just inform the user. I suppose that there are many invalid files out there. Probably also, just because TiGL accepts them anyhow.

bernhardmgruber commented 7 years ago

Then we will take version 2.