Skip to content

Commit

Permalink
Entry Model Tweaks (#579)
Browse files Browse the repository at this point in the history 
* Reset boundaries whenever the base is changed.

* Add support for converting the value to the new base.

* Don't clamp if the value is 0 and there is space for more leading zeros.

* Temove redundant boundary check.

* Handle the case of going from leading zeros to non-zero push_back().

* handle leading zeros correctly for pop_back().

* Adjust for leading zeros when recalculating size.

* Properly handle leading zeros for negative numbers.

* Remove unnecessary assert.

* Add tests.

* Fix review comments.
  • Loading branch information
@bakerstu
bakerstu authored Oct 9, 2021
1 parent d9ae5f7 commit 1a1b9f8
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 19 deletions.
121 changes: 120 additions & 1 deletion src/utils/EntryModel.cxxtest
View file
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());
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
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

0 comments on commit 1a1b9f8

Please sign in to comment.