Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entry Model Tweaks #579

Merged
merged 11 commits into from
Oct 9, 2021
Merged

Entry Model Tweaks #579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7a9c74b
Reset boundaries whenever the base is changed.
bakerstu Oct 8, 2021
2c969e4
Add support for converting the value to the new base.
bakerstu Oct 9, 2021
d236aba
Don't clamp if the value is 0 and there is space for more leading zeros.
bakerstu Oct 9, 2021
cd8d711
Temove redundant boundary check.
bakerstu Oct 9, 2021
eae0557
Handle the case of going from leading zeros to non-zero push_back().
bakerstu Oct 9, 2021
edb5164
handle leading zeros correctly for pop_back().
bakerstu Oct 9, 2021
5485525
Adjust for leading zeros when recalculating size.
bakerstu Oct 9, 2021
e55bd28
Properly handle leading zeros for negative numbers.
bakerstu Oct 9, 2021
a67f5d6
Remove unnecessary assert.
bakerstu Oct 9, 2021
81ffb9f
Add tests.
bakerstu Oct 9, 2021
4d93fc0
Fix review comments.
bakerstu Oct 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 120 additions & 1 deletion src/utils/EntryModel.cxxtest
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,23 +166,95 @@ TEST(EntryModelTest, InitValuePopBack)
// unsigned 0 value with leading zeros
uem.init(4, 10, 0);

EXPECT_EQ(4U, em.max_size());
EXPECT_EQ(4U, uem.max_size());
uem.push_back_char('0');
EXPECT_EQ(0U, uem.get_value());
EXPECT_EQ("0", uem.get_string());
EXPECT_EQ(" 0", uem.get_string(true));
EXPECT_FALSE(uem.has_leading_zeros());
uem.push_back(0);
EXPECT_EQ(0U, uem.get_value());
EXPECT_EQ("00", uem.get_string());
EXPECT_EQ(" 00", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.pop_back();
EXPECT_EQ(0U, uem.get_value());
EXPECT_EQ("0", uem.get_string());
EXPECT_EQ(" 0", uem.get_string(true));
EXPECT_FALSE(uem.has_leading_zeros());
uem.pop_back();
EXPECT_EQ("", uem.get_string());
EXPECT_EQ(" ", uem.get_string(true));
EXPECT_FALSE(uem.has_leading_zeros());
EXPECT_TRUE(uem.empty());

// unsigned 0 value with leading zeros, non-zero result
uem.init(4, 10, 0);

EXPECT_EQ(4U, uem.max_size());
uem.push_back_char('0');
EXPECT_EQ(0U, uem.get_value());
EXPECT_EQ("0", uem.get_string());
EXPECT_EQ(" 0", uem.get_string(true));
EXPECT_FALSE(uem.has_leading_zeros());
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider adding expectations on uem.get_value() as well in these steps

Copy link
Owner Author

Choose a reason for hiding this comment

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

Fixed.

uem.push_back(0);
EXPECT_EQ(0U, uem.get_value());
EXPECT_EQ("00", uem.get_string());
EXPECT_EQ(" 00", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.push_back(5);
EXPECT_EQ(5U, uem.get_value());
EXPECT_EQ("005", uem.get_string());
EXPECT_EQ(" 005", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.pop_back();
EXPECT_EQ(0U, uem.get_value());
EXPECT_EQ("00", uem.get_string());
EXPECT_EQ(" 00", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.pop_back();
EXPECT_EQ(0U, uem.get_value());
EXPECT_EQ("0", uem.get_string());
EXPECT_EQ(" 0", uem.get_string(true));
EXPECT_FALSE(uem.has_leading_zeros());
}

TEST(EntryModelTest, SetBaseConvert)
{
EntryModel<int64_t> em;
EntryModel<uint64_t> uem;

// signed
em.init(4, 10, -43);
EXPECT_EQ(-43, em.get_value());
EXPECT_EQ("-43", em.get_string());
EXPECT_EQ(" -43", em.get_string(true));

em.set_base(16, true);
EXPECT_EQ(-0x43, em.get_value());
EXPECT_EQ("-43", em.get_string());
EXPECT_EQ(" -43", em.get_string(true));

em.set_base(10, true);
EXPECT_EQ(-43, em.get_value());
EXPECT_EQ("-43", em.get_string());
EXPECT_EQ(" -43", em.get_string(true));

// unsigned
uem.init(4, 10, 34);
EXPECT_EQ(34U, uem.get_value());
EXPECT_EQ("34", uem.get_string());
EXPECT_EQ(" 34", uem.get_string(true));

uem.set_base(16, true);
EXPECT_EQ(0x34U, uem.get_value());
EXPECT_EQ("34", uem.get_string());
EXPECT_EQ(" 34", uem.get_string(true));

uem.set_base(10, true);
EXPECT_EQ(34U, uem.get_value());
EXPECT_EQ("34", uem.get_string());
EXPECT_EQ(" 34", uem.get_string(true));
}

TEST(EntryModelTest, PushBackAndAppend)
Expand Down Expand Up @@ -501,3 +573,50 @@ TEST(EntryModelBoundedTest, SetMinMax)
EXPECT_FALSE(em.is_at_initial_value());
EXPECT_FALSE(em.empty());
}

TEST(EntryModelBoundedTest, LeadingZerosNonZeroMin)
{
EntryModelBounded<uint16_t> uem;

// unsigned 0 value with leading zeros
uem.init(4, 10, 5, 5, 9999, 5);
uem.clear();

EXPECT_EQ(4U, uem.max_size());
uem.push_back_char('0');
EXPECT_EQ("0", uem.get_string());
EXPECT_EQ(" 0", uem.get_string(true));
EXPECT_EQ(1U, uem.size());
EXPECT_FALSE(uem.has_leading_zeros());
uem.push_back(0);
EXPECT_EQ("00", uem.get_string());
EXPECT_EQ(" 00", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.push_back(0);
EXPECT_EQ("000", uem.get_string());
EXPECT_EQ(" 000", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.push_back(0);
EXPECT_EQ("0005", uem.get_string());
EXPECT_EQ("0005", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());

uem.pop_back();
EXPECT_EQ("000", uem.get_string());
EXPECT_EQ(" 000", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.pop_back();
EXPECT_EQ("00", uem.get_string());
EXPECT_EQ(" 00", uem.get_string(true));
EXPECT_TRUE(uem.has_leading_zeros());
uem.pop_back();
EXPECT_EQ("0", uem.get_string());
EXPECT_EQ(" 0", uem.get_string(true));
EXPECT_FALSE(uem.empty());
EXPECT_FALSE(uem.has_leading_zeros());
uem.pop_back();
EXPECT_EQ("", uem.get_string());
EXPECT_EQ(" ", uem.get_string(true));
EXPECT_TRUE(uem.empty());
EXPECT_FALSE(uem.has_leading_zeros());
}
67 changes: 49 additions & 18 deletions src/utils/EntryModel.hxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ public:
{
maxSize_ = max_size;
clear();
set_base(base);
set_boundaries();
set_base(base); // this will call set_boundaries()
}

/// Initialize with a value.
Expand Down Expand Up @@ -111,6 +110,13 @@ public:
// clear entry before inserting character
clear();
}
if (value_ == 0 && val != 0 && size_)
{
// This is a special case where we transition from a user having
// entered all 0 (as a first digits) and then enters a non-zero
// (as a next digit).
++numLeadingZeros_;
}
value_ *= base_;
if (value_ < 0)
{
Expand Down Expand Up @@ -139,13 +145,10 @@ public:
switch (base_)
{
case 10:
HASSERT(c >= '0' && c <= '9');
push_back(c - '0');
break;
case 16:
c = toupper(c);
HASSERT((c >= '0' && c <= '9') ||
(c >= 'A' && c <= 'F'));
push_back(c <= '9' ? c - '0' : c - 'A' + 10);
break;
}
Expand All @@ -172,14 +175,11 @@ public:
/// Removes (deletes) a character off the end.
void pop_back()
{
value_ /= base_;
if (value_ == 0 && numLeadingZeros_)
{
--numLeadingZeros_;
}
else
{
value_ /= base_;
}
if (size_)
{
--size_;
Expand All @@ -205,6 +205,30 @@ public:
{
HASSERT(base == 10 || base == 16);
base_ = base;
set_boundaries();
}

/// Set the radix base.
/// @param base new radix base to set.
/// @param convert convert the current value, as a string, to the new base.
void set_base(int base, bool convert)
{
if (base != base_)
{
if (convert)
{
string str = get_string();
if (std::is_signed<T>::value)
{
value_ = strtoll(str.c_str(), nullptr, base);
}
else
{
value_ = strtoull(str.c_str(), nullptr, base);
}
}
set_base(base);
}
}

/// Set the value, keep the max number of digits and base the same.
Expand Down Expand Up @@ -314,14 +338,10 @@ public:
break;
}
}
// Assert that we do not have more leading zeros than space allows.
// The logic of push_back should never allow it.
HASSERT(numLeadingZeros_ == 0 ||
(str.size() + numLeadingZeros_) <= maxSize_);

if (numLeadingZeros_)
{
str.insert(0, numLeadingZeros_, '0');
str.insert(value_ < 0 ? 1 : 0, numLeadingZeros_, '0');
}
if (right_justify && str.size() < maxSize_)
{
Expand Down Expand Up @@ -357,14 +377,21 @@ public:
clamp();
}

/// Clamp the value at the min or max.
/// Clamp the value at the min or max. Clamping will not occur if the value
/// is zero and there is space for more leading zeros.
/// @param force Normally, clamping doesn't occur if the entry is "empty".
/// However, if force is set to true, we will clamp anyways.
/// force also applies if the value is zero yet there is space
/// for more leading zeros.
virtual void clamp(bool force = false)
{
if (value_ == 0 && size_ < maxSize_ && !force)
{
// skip clamping if we have space for more leading zeros
return;
}
if (force || !empty_)
{
// purposely do not reset the numLeadingZeros_
empty_ = false;
if (value_ < valueMin_)
{
Expand Down Expand Up @@ -424,11 +451,14 @@ protected:
void calculate_size()
{
// calculate new size_
size_ = value_ < 0 ? numLeadingZeros_ + 1 : numLeadingZeros_;
size_ = value_ < 0 ? 1 : 0;
for (T tmp = value_ < 0 ? -value_ : value_; tmp != 0; tmp /= base_)
{
++size_;
}
numLeadingZeros_ = std::min(static_cast<unsigned>(numLeadingZeros_),
static_cast<unsigned>(maxSize_ - size_));
size_ += numLeadingZeros_;
}

T value_; ///< present value held
Expand Down Expand Up @@ -481,7 +511,8 @@ public:
}

private:
/// Clamp the value at the min or max.
/// Clamp the value at the min or max. Clamping will not occur if the value
/// is zero and there is space for more leading zeros.
/// @param force Normally, clamping doesn't occur if the entry is "empty".
/// However, if force is set to true, we will clamp anyways.
void clamp(bool force = false) override
Expand Down