BT 2.3.1: BeerXML output does not contain estimated IBU value

[zigurana]

Ok, so this is probably a stupid question, but why is there not an entry for the (estimated) IBU level of my beer in the exported BeerXML?

As far as I can see, there were no problems for any of the other basic beer parameters (OG, FG, dates, ingredients, etc).

Thanks,

[JulianVolodia]

Could you provide steps? Maybe just lack of feature and could be easily refactored?

Have you tried to debug it?

I ask bc it is great if you contribute in this project!

[matty0ung]

I haven't done a deep dive on this, but, from a quick look, I think the issue is that the overloaded version of BeerXML::toXml() that deals with Recipes just runs through all the database columns of the Recipe table to get primary attributes, but IBU is not stored here as it is calculated on the fly (by Recipe::recalcIBU()), and so doesn't get included in the generated XML.

Two approaches for fixing seem possible. A small, but slightly ugly, patch would just add some extra special case code to BeerXML::toXml() to deal with calculated attributes. A more elegant solution might be to change the XML generation approach from delving around in the Database to instead using the existing Q_PROPERTY attributes of Recipe (which already include the ability to read IBU). This would increase modularity, but I'd like to get some input from @mikfire before doing such a refactor.

[mikfire]

Oddly enough, there is no IBU field defined in the RECIPE XML specification and so it has never been implemented. @rocketman768 was pretty strict in how he followed the spec, and I've only recently begun to really ignore it. I might even go so far as to say this isn't a bug, but a feature request.

My impulse is to simply modify the proper BeerXML::toXML method and call it a day. It's one field, it isn't in the specification and let's not get complex until we need to. It breaks a lot of other work and makes me just a little nervous.

The more complete solution would be to extend the recipe schema so that we do store it, and the proper modifications to TableSchema.

An option perhaps worth considering would be to see what happens when a TableSchema entry defines an attribute and an xml attribute but not a column name.

I would prefer to avoid the Q_PROPERTY approach. The recipe object shouldn't have to know what database it is writing to. My concern is that the Q_PROPERTY approach will force that knowledge back into the object.

[matty0ung]

Ah, fair enough if it's not in the spec. Perhaps should therefore be a feature request to whoever is guardian of the BeerXML spec. (I struggled a bit to find definitive latest info via Google as the link to 'discussion about v2 of the standard' seems a bit broken - http://beerxml.com/forum/)

On the Q_PROPERTY thing, I'm not sure I understand how it links us the database (which I agree would be a bad thing). It may be my unfamiliarity with the code, but I thought Q_PROPERTY declarations in recipe.h and similar places just provide type introspection. Even if Database class uses Q_PROPERTY calls to access getters/setters on Recipe, is it not still Recipe that "owns" such getters and setters?

[matty0ung]

We have reworked the way BeerXML is written as part of #617, so we could look at this again. Having now spent somewhat more time looking at the BeerXML specs, I see that IBU is an "extension tag" on Recipe that programs may optionally support for export "to enhance the easy display of data" but should not use for import.

The Recipe code already knows how to return the recipe's IBU (as a read-only value). And the XML import process will already ignore fields that we don't need/want. (Specifically, Recipe's constructor will ignore any fields it doesn't need in the NamedParameterBundle supplied to it from the XML code.)

Long story short, I believe this will be a one-line fix once #617 is merged.

[matty0ung]

I have implemented this in Brewken and it was sort-of a one line fix -- one line of code plus a few lines of comments! 😄

[mattiasmaahl]

included in PR #629, please reopen if need be.