This repository was archived by the owner on Aug 5, 2022. It is now read-only.
Criterion rework #96
Merged
Criterion rework #96
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
clero
force-pushed
the
criterion_rework
branch
8 times, most recently
from
5046be7 to
d586479
Compare
clero
force-pushed
the
criterion_rework
branch
4 times, most recently
from
db4254d to
4e992b5
Compare
bindings/c/ParameterFramework.cpp
Outdated
Contributor
There was a problem hiding this comment.
You are hardly using this typedef.
clero
force-pushed
the
criterion_rework
branch
4 times, most recently
from
ba3a527 to
8121cb1
Compare
parameter/SelectionCriterionRule.cpp
Outdated
Contributor
Author
There was a problem hiding this comment.
need to handle "none" case
Contributor
There was a problem hiding this comment.
these usings could be moved outside of the class and directly in the core::criterion namespace; this would make it easier to use.
Previously, numerical values of inclusive and standard (exclusive)
criterion was not handled in the same way.
Exclusive criterion can have any integer as numerical value. On
the contrary, inclusive criterion can only have a numerical state
which is a power of 2. It comes from the fact that a bit mask is
internally used to handle inclusive criterion state.
This implementation forces the client to register criteria with
a mask where only one bit is set.
For example, to register a literal 'A' in the third position of
a bit mask we have to do criterion.addValuePair('A', 1 << 3).
This is an error prone interface because it is not consistent
with the exclusive criterion interface.
This patch rework this interface to register the literal value
with the desired bit mask index (e.g: criterion.addValuePair('A', 3)).
Signed-off-by: Jules Clero <julesx.clero@intel.com>
Signed-off-by: Jules Clero <julesx.clero@intel.com>
In previous API, criterion values were added through addValuePair interface. This API was error prone because we were able to create bad criterion (with no value for example). Moreover, the default criterion state was '0' which is not always a valid state for an exclusive criterion which could leads to some problems at startup. The new criterion creation API requires that the client provide a Values object which contains pairs the criterion will deal with. Some check are done on this object to insure the criterion will be valid. The default state is the first one added to the Values object. Signed-off-by: Jules Clero <julesx.clero@intel.com>
…riterion Inclusive criterion has their getNumericalValue specialized in order to format a bit mask corresponding to the literal value given in argument. This value can contains several '|' to combine values which will composed the returned mask. This is not consistent with its twin method getLiteralValue which can only take an 'atomic' numerical value in parameter. Moreover, getNumericalValue is not returning a 'numerical value' but rather a 'criterion state'. This patch removes getNumericalValue specialization for inclusive criterion. The method called is now the one implemented in Criterion class and has the same behaviour. Signed-off-by: Jules Clero <julesx.clero@intel.com>
Criterion API was allowing to set any state we want to any criterion. This patch rework this API in order to add some check when setting a criterion state. It is now required that all parts of the state we set are registered values. Moreover inclusive criterion can now handle as many value as an exclusive criterion (e.g as many values as an signed integer can take). Signed-off-by: Jules Clero <julesx.clero@intel.com>
Criterion default value handling has been enhanced. Now, that inclusive criterion is not represented by bit mask, 0 value can be used as any other numerical value. The "none" state of a criterion is now represented as an empty state. This special state does not comes as any other standard, user provided value, so it is not displayed as an available value. Exclusive criterion default value handling remains the same, the first value provided by the user is the default one. This value can be set either explicitly or by using an empty state. Signed-off-by: Jules Clero <julesx.clero@intel.com>
Current criterion interface allows user to register a pair of numerical/literal value. Then the user must use numerical values to set the desired state. This patch removes this possibility. The user is now registering only literal values which are used to set the desired state. The mapping between numerical and literal value, if needed, should now be handled by the client. Moreover, Values are now given by the client through a list not a set. It allows the client to have the control of the default state which is the first value of the list. Internally, we still use a set to avoid value duplication. Signed-off-by: Jules Clero <julesx.clero@intel.com>
Some methods has not be touched during the refactor. This patch introduces a documentation for those methods. Signed-off-by: Jules Clero <julesx.clero@intel.com>
Some methods have kept their old name (with Selection or Criterion prefix). This is the same thing for Criterion XML tag. This patch unify them by removing those prefix. Signed-off-by: Jules Clero <julesx.clero@intel.com>
Previously, client was sometimes needing to know either the criterion was Inclusive or Exclusive. Now that setState API has been secured, the client do not have to realize any check. Moreover, if it really wants it, he can keep it after the criterion creation. This patch removes the isInclusive API. Signed-off-by: Jules Clero <julesx.clero@intel.com>
getFormattedState allows a client to retrieve properly a formatted string containing all current state value in a formatted way. Previous implementation was needing this helper as the state translation to literal values was not straightforward. Now that state is just a set of string, the client can easily display this information and we don't need to impose him a format. The method remains private as it is used for display purpose. Signed-off-by: Jules Clero <julesx.clero@intel.com>
Some code has not been touched by the rework. Thus, old naming convention can still be found at some places. This patch correct concerned variable name. Signed-off-by: Jules Clero <julesx.clero@intel.com>
CriterionInterface is not a convenient name to use. This patch rename this interface Criterion. It also introduces the internal namespace to avoid conflict between client Criterion API and the one used internally which is also named Criterion. Signed-off-by: Jules Clero <julesx.clero@intel.com>
Criterion handling code contains a lot of copy pasted lines. This patch rework the criterion creation handling to avoid it. Signed-off-by: Jules Clero <julesx.clero@intel.com>
dawagner
added a commit
that referenced
this pull request
Jun 15, 2015
Criterion API rework Previous criterion API was error prone in many point: - exposition of internal components of a criterion - no check when setting a criterion - inclusive criterion were limited to 31 values - no check for inclusive criterion limit - Liskov principle was broken due to isInclusive method This rework introduces a new criterion API which corrects those issues.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #93
This is a rework of Pfw criterion. Please do not merge now but some feedback on designs changes would be appreciate.
Next step:
Update coding style everywhere
Some documentation is still missing