Skip to content
This repository was archived by the owner on Aug 5, 2022. It is now read-only.

Conversation

dawagner
Copy link
Contributor

@dawagner dawagner commented Oct 4, 2016

Make IntegerParameterType template over the size and signedness - simplifies the implementation and makes the code more robust.

@@             master       #376   diff @@
==========================================
Files           211        213     +2
Lines          6683       6667    -16
Methods        1438       1445     +7
Messages          0          0
Branches        857        844    -13
==========================================
+ Hits           4814       4815     +1
+ Misses         1397       1385    -12
+ Partials        472        467     -5

Powered by Codecov. Last update b871a8f...9cf536a

  

Remove convertValueFromString and replace it with converTo.

Signed-off-by: David Wagner <david.wagner@intel.com>
@dawagner
Copy link
Contributor Author

dawagner commented Oct 4, 2016

@krocard @sguiriec Please review. @sguiriec : This should solve #375 (except the min > max error, which can be easily fixed afterward).

@codecov-io
Copy link

codecov-io commented Oct 4, 2016

Current coverage is 72.29% (diff: 74.82%)

Merging #376 into master will increase coverage by 0.26%

@@             master       #376   diff @@
==========================================
  Files           211        214     +3   
  Lines          6683       6674     -9   
  Methods        1438       1447     +9   
  Messages          0          0          
  Branches        857        845    -12   
==========================================
+ Hits           4814       4825    +11   
+ Misses         1397       1382    -15   
+ Partials        472        467     -5   

Powered by Codecov. Last update b871a8f...9199e04

This simplifies some methods that used to contain if/else branches over
the signedess. It also simplifies how different sizes are handled (gets
rid of some bit-level tricks).

As a result, it isn't possible anymore to set illegal Min/Max
attributes.

Signed-off-by: David Wagner <david@marvid.fr>
@dawagner dawagner force-pushed the integerparam-template branch from 9cf536a to cba27ea Compare October 5, 2016 16:10
@sguiriec
Copy link

sguiriec commented Oct 7, 2016

@dwagner The code is solving Mix/Max boundaries but not Min > Max doesn't generate Exception or FAIL. So need to be corrected.

To test it you can cherry pick b0ccc19 and then change Execption cantch in std::domain_error.

Then you will see that we can import XML with Min > Max

Copy link

@sguiriec sguiriec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add Min > Max error. Otherwise it is OK

@dawagner
Copy link
Contributor Author

dawagner commented Oct 7, 2016

@sguiriec yes, that's expected (cf. the commit message) - I intended to validate the first fix first. I'll send an additional patch for this soon.

@dawagner dawagner force-pushed the integerparam-template branch from 1ab294b to 7a1d56c Compare October 7, 2016 14:28
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2015, Intel Corporation

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess should be 2016

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed

dawagner and others added 3 commits October 18, 2016 10:08
And catch the exception in CElement::fromXml.

Signed-off-by: David Wagner <david.wagner@intel.com>
Otherwise, error out and inform the user that the range of allowed
values is empty.

Signed-off-by: David Wagner <david.wagner@intel.com>
Tests that Min and Max boudaries are correctly checked.

Signed-off-by: Sebastien Guiriec <sebastien.guiriec@intel.com>
Signed-off-by: David Wagner <david.wagner@intel.com>
@dawagner dawagner force-pushed the integerparam-template branch from 7a1d56c to 9199e04 Compare October 18, 2016 08:08
@dawagner dawagner merged commit a99e07d into intel:master Oct 20, 2016
@dawagner dawagner deleted the integerparam-template branch October 20, 2016 08:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants