From kde-commits Thu Apr 07 20:25:06 2016 From: =?utf-8?q?Jos=C3=A9_Manuel_Santamar=C3=ADa_Lema?= Date: Thu, 07 Apr 2016 20:25:06 +0000 To: kde-commits Subject: [krecipes] src/api_v2_design: Add draft for a new API Message-Id: X-MARC-Message: https://marc.info/?l=kde-commits&m=146006071804694 Git commit df1e1822762d9be62944c1d7031794cb0d80f0e9 by Jos=C3=A9 Manuel San= tamar=C3=ADa Lema. Committed on 07/04/2016 at 06:31. Pushed by joselema into branch 'master'. Add draft for a new API A +198 -0 src/api_v2_design/README A +73 -0 src/api_v2_design/kreelements.h [License: UNKNOWN] * A +76 -0 src/api_v2_design/kremodels.h [License: UNKNOWN] * The files marked with a * at the end have a non valid license. Please read:= http://techbase.kde.org/Policies/Licensing_Policy and use the headers whic= h are listed at that page. http://commits.kde.org/krecipes/df1e1822762d9be62944c1d7031794cb0d80f0e9 diff --git a/src/api_v2_design/README b/src/api_v2_design/README new file mode 100644 index 0000000..2daec6a --- /dev/null +++ b/src/api_v2_design/README @@ -0,0 +1,198 @@ +------------------------------ +Krecipes API and design issues +------------------------------ + +There are various flaws in the Krecipes API, i.e. the classes inside these +directories: +- src/datablocks/ +- src/backends/ +- src/importers/ +- src/exporters/ + +List of issues: + +#1 The handling of database Id's: + - The ids are int's or, in the best case of type RecipeDB::IdType, whic= h is a + typedef for int. According to the SQLite documentation the numbers us= ed for + primary keys are 64-bit signed integers, so, if I'm not mistaken it's= not + guaranteed that the database id will fit in the int. + - Also having the database Id's as numbers makes more difficult to hand= le + foreign keys which are set to NULL. Usually this is handled as -1 or + RecipeDB::InvalidId (which is -1), not only in the program logic, but= also + on the database itself; sometimes external keys are stored as -1 whil= e = + actually they should be stored as NULL + +#2 Classes inside src/datablocks doesn't have getters or setters except fo= r Unit + +#3 The class Ingredient in src/datablocks is not a class to represent a ro= w in + the 'ingredients' SQL table; it's a class to represent an ingredient en= try + for a recipe. To represent a row in the ingredients table the class Ele= ment. + This various issues: + - It's confusing + - It's difficult to reference ingredient subsitutes (see the Ingredient= vs + IngredientData hack to have a list of substitutes) + - As mentioned the class Element is used to represent a row of the + ingredients table, consider this RecipeDB signal: + void ingredientCreated( const Element & ); + So what happens if I want to change the program and database schema to + support singular/plural names? I would have to create a new class to + represent a row in the 'ingredients' table, let's call it 'Ingredient= Row' + and change the api of RecipeDB changing the signal mentioned above li= ke + this: + void ingredientCreated( const IngredientRow & ); + It would have been better to have a class to represent row from the t= able = + 'ingredients' (even if it were an empty subclass of the 'Element' cla= ss). + +#4 Right now, Krecipes has an option to limit the number of elements to sh= ow; + configuring this option has this effect: program dialogs shown when + clicking "Data..." in the left panel display the items in a "paged" fas= hion; + a couple of buttons appear in the bottom of the QTreeView to go to the = next + page or the previous page. + However this is kind of futile, because when we edit a recipe we = + are reading all the ingredients, units and preparation methods of the + database for autocompletion objects and comboboxes in the recipe editor. + So maybe it would be nice to have something to load all the external el= ements + needed for a recipe just once when the program starts and then use this + elements. + Viewing the things under "Data..." in a paged fashion has also other is= sues: + - When creating a new element there is no other way to update the QTree= View + properly than reloading the current "page": when we insert a new elem= ent, + for example, there is no way to know properly the insertion point bec= ause + the sorting algorigthm of the database and the sorting algorithm of + QSortFilterProxyModel could differ. This means that when we import re= cipes + from a file and we need to create new ingredients/authors/whatever we + reload the "Data..." -> Ingredients/Authors/whatever QTreeView. + - Since the QTreeview's under the "Data..." are using a model containin= g only + the elements of the current page the search text box returns the matc= hes + only for the current page and not for the whole database (which would= be + more appropiate). + +#5 Viewing/exporting recipes without saving them in the database: + See this bug report: https://bugs.kde.org/show_bug.cgi?id=3D292099 + Right now, there is no way to display a recipe wihtout saving it; if yo= u edit + a recipe and you click "Show Recipe" you will be asked to save the reci= pe in + the database before displaying it; if you don't save, you won't be able= to + view the modified recipe. + To view a recipe it's exported to HTML by the "XSLT exporter" to a temp= orary + directory under "/tmp" and then the resulting HTML is displayed in a KH= TML + part. + So maybe it would be nice to load into memory everything we would need = to + export a recipe so the exporters would take a recipe object without + accessing the database at all. + +#6 Use of QLinkedList: sometimes QLinkedList is used to transfer data from= the + database, this is because sometimes we need persistent iterators to pop= ulate + some classes from 'src/datablocks' with data, see for instance the log = of + the commit e5ceac80bdd0c9ac4e4d5178f98345c17021b4f1 + "RecipeList should be a subclass of QLinkedList because in + QSqlRecipeDB::loadRecipes method we are keeping iterators to RecipeLi= st + elements in recipeIterators hasmap after doing non-const operations a= gainst + the RecipeList. This is wrong if RecipeList is a QList but fine if + RecipeList is a QLinkedList. + This commit fixes some random segfaults when showing recipes and when= using + the ingredient matcher." + + +---------------------------------------- +New API design to solve the above issues +---------------------------------------- + +* Solution for #1: use QVariant's to store id's. This way we will be able = to + manage NULL foreign keys with QVariant::isNull() and also it will be + guaranteed that the integer type used by the DBMS for id's will fit in o= ur id + variables. + +* Solution for #2: make getters and setters in the new classes instead of = having + public data members. + +* Solution for #3: create two separate classes: + - KreIngredient to represent an ingredient object i.e. a row in the + 'ingredients' SQL table + - KreRecipeIngredientEntry to represent an ingredient entry of a recipe;= a + recipe would have from 0 to n ingredient entries. + +* Solution for #4 and #5: keep some global models in RecipeDB; this models + would be updated automatically with changes in the database connecting + themselves against RecipeDB. For instance we could have a class named + KreAllIngredientsModel providing a set of models which would be populated + by RecipeDB and updated connecting to recipeDB signals. Regarding the is= sue + #5 "Viewing/exporting recipes without saving them in the database" see b= elow + the section "Data needed to edit/view/export a recipe without saving it"= for + more details. Also this global models could be used as a cache for the + database. + - NOTE 1: if we have a QStandardItemModel for all ingredients with 2 col= umns: + id and ingredient name we could filter out the id one for comboboxes u= sing + this technique: + https://wiki.qt.io/Filter_Columns_in_a_QFileSystemModel* = + - NOTE 2: To transfer data from the database QSqlRecipeDB will use eithe= r: + a) dtos or QLists of dtos + b) models such as QStandardItemModel or KCompletion objects + For instance, to read all the ingredients from the database we will us= e the + approach b) for efficiency reasons. However to load the needed data to= edit + a recipe we will use the the approach b) and then we will use the data= from + the recipe DTO (a KreRecipe object) to populate RecipeGeneralInfoEdito= r or + the model of IngredientsEditor. + +* Solution for #6: Use QList instead of QLinkedList and avoid the use of + non-const iterators to alter lists as much as possible. Probably, this w= ill + reduce the number of mallocs when reading stuff from the database. + + +------------------------ +Mockup of the new design +------------------------ + +* Inside this directory there is a file called 'datablocks_v2.h' with some + drafts of the classes meant to replace the current stuff in 'src/datablo= cks'. +* There is also a file called 'api_v2_models.h' with some drafs of the mod= el + classes meant to be filled by RecipeDB and used all over the program when + needed. +* The classes in 'datablocks_v2.h' would be located in 'src/dtos/'. DTO me= ans + "Data Transfer Object" = +* The classes in 'api_v2_models.h' would be located mostly in 'src/models/' + + +---------------------------------------------------------- +Data needed to edit/view/export a recipe without saving it +---------------------------------------------------------- + +A recipe would be represented by the KreRecipe class; this class should be +populated with all the "external data" needed to edit/view/export a recipe. + +The table below lists the "external data" needed and where we could get it= . All +that data should be stored in the recipe DTO (the KreRecipe class). + ++-------------------------------------------------------------------------= -------------+ +| DATA NEEDED | WE COULD GET IT FROM: = | ++-------------------------------------------------------------------------= -------------+ +| Name of the ingredients used | KreAllIngredientsModels global obje= ct | ++-------------------------------------------------------------------------= -------------+ +| Name of the headers used | KreAllHeadersModels global object = | ++-------------------------------------------------------------------------= -------------+ +| Name singular/plural/abbreviations | KreAllUnitsModels global object = | +| of the units used | = | ++-------------------------------------------------------------------------= -------------+ +| Name of the preparation methods | KreAllPrepMethodsModels global obje= ct | ++-------------------------------------------------------------------------= -------------+ +| Properties of the ingredients used | The database: when viewing/editing = a recipe we | +| | should have a map indexed by = | = +| | ingredient id like this: = | +| | QHash | ++-------------------------------------------------------------------------= -------------+ +| Weights of the ingredients used | The database: when viewing/editing = a recipe we | +| | should have a map indexed by = | +| | ingredient id like this: = | +| | QHash | ++-------------------------------------------------------------------------= -------------+ +| Unit conversions | KreUnitConversionsModel global obje= ct | ++-------------------------------------------------------------------------= -------------+ +| Category names | KreAllRecipesTreeModel global objec= t | ++-------------------------------------------------------------------------= -------------+ +| Names of the authors | KreAllAuthorsModels global object = | ++-------------------------------------------------------------------------= -------------+ +| Ratings | The database = | ++-------------------------------------------------------------------------= -------------+ +| Yield type | KreAllYieldsTypesModels global obje= ct | ++-------------------------------------------------------------------------= -------------+ + + diff --git a/src/api_v2_design/kreelements.h b/src/api_v2_design/kreelement= s.h new file mode 100644 index 0000000..3222d95 --- /dev/null +++ b/src/api_v2_design/kreelements.h @@ -0,0 +1,73 @@ +class KreElement { + public: + KreElement(); + KreElement( const QVariant & id, const QString & name ); + + QVariant id() const; + void setId( const QVariant & id ); + + QString name() const; + void setName( const QString & name ); + + private: + QVariant id; + QVariant name; +}; + +class KreIngredient: public KreElement { + public: + KreIngredient(); + KreIngredient( const QVariant & id, const QString & name ); +} + +class KreIngHeader: public KreElement { + public + KreIngHeader(); + KreIngHeader( const QVariant & id, const QString & name ); +} + +class KreUnit: public KreElement { + ... +} + +class KreIngredientPropertyEntry { + public: + KrePropertyEntry(); + + QVariant propertyId(); + void setPropertyId( const QVariant & propertyId ); + + double amount(); + void setAmount( double amount ); + + QVariant unitId() + void setUnitId( const QVariant & unitId ); + + private: + ... +} + +class KreRecipeIngredientEntry { + public: + KreIngredientEntry(); + KreIngredientEntry( const KreIngredient & ingredient, + const KreMixedNumberRange & amount, + const KreUnit & unit, + const QList & prepMethods, + const QList & substitutesList =3D QList(), + const KreIngHeader & ingHeader =3D KreIngHeader() ); + ... +} + +class KreRecipe: public KreElement { + public: + ... + QList ingredientEntryList() const; + void addIngredientEntry( const KreIngredientEntry & ingredientEntry ); + ... + + private: + ... + QList m_ingredientEntryList; + ... +} diff --git a/src/api_v2_design/kremodels.h b/src/api_v2_design/kremodels.h new file mode 100644 index 0000000..8d9374c --- /dev/null +++ b/src/api_v2_design/kremodels.h @@ -0,0 +1,76 @@ +template class KreAllElementsModels { + public: + KreGenericModel( RecipeDB * database ); + + QStringListModel * nameModel(); + QSortFilterProxyModel * nameModelSorted(); + KCompletion * nameCompletion(); + + QList idsForName( const QString & name ); + + protected: + QHash m_idToElementMap; + QMultiHash m_nameToIdMap; + + QStringListModel * m_nameModel; + QSortFilterProxyModel * m_nameModelSorted; + KCompletion * m_nameCompletion; +} + + +class KreAllIngredientsModels: public KreGenericModels { + public: + KreIngredientModel( RecipeDB * database ); + +} + +class KreAllHeadersModels + +class KreAllUnitsModels: public KreAllElementsModels { + public: + KreAllUnitsModels( RecipeDB * database ); + + QStringListModel * pluralNameModel(); + QSortFilterProxyModel * pluralNameModelSorted(); + KCompletion * pluralNameCompletion(); + = + QList idsForPlural( const QString & pluralName ); + + protected: + QMultiHash m_pluralNameToIdMap; + + QStringListModel * m_pluralNameModel; + QSortFilterProxyModel * m_pluralNameModelSorted; + KCompletion * m_pluralNameCompletion; +} + +class KreAllMassUnitConversionsModel +class KreAllVolumeConversionsModel + +class KreAllPrepMethodsModels + +class KreAllPropertiesModels + +class KreAllAuthorsModels + +class KreRecipeTreeModel .... //would store the categories/recipe tree + +//Model to get the set of properties for a given ingredient +class KrePropertiesForIngredientModel { + public: + KrePropertiesForIngredientModel( RecipeDB * database. + const QVariant & ingredientId ); + + ... + private: + QHash m_propertyIdToEntryMap; +} + +//Similar ro KrePropertiesForIngredientModel +class KreWeightsForIngredientModel + +class KreRecipeIngredientsEditorModel { + ... + private: + QHash m_ingredientIdToProperti= esMap; +}