Skip to content

Commit

Permalink
(SW-8261) Avoid empty value as first element of native buffers (#94)
Browse files Browse the repository at this point in the history 
* Stop adding initial empty record to circular buffer
  • Loading branch information
@dskyle
dskyle committed Sep 24, 2018
1 parent 7f28aef commit 581a9a8
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 69 deletions.
82 changes: 54 additions & 28 deletions include/madara/knowledge/KnowledgeRecord.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ KnowledgeRecord::to_file (const std::string & filename) const
}
else if (has_history ())
{
if (buf_->empty ())
{
buf_->emplace_back ();
}
return ref_newest ().to_file (filename);
}
else
Expand All @@ -155,13 +159,17 @@ KnowledgeRecord::to_double (void) const
{
double value = 0;

if (!exists ()) {
return value;
}

if (type_ == DOUBLE)
value = double_value_;
else if (type_ == DOUBLE_ARRAY)
value = double_array_->size () == 0 ? 0 : double_array_->at(0);
else if (has_history())
return ref_newest ().to_double ();
else if (type_ != EMPTY)
else if (has_history ())
value = ref_newest ().to_double ();
else
{
std::stringstream buffer;

Expand All @@ -184,13 +192,17 @@ KnowledgeRecord::to_integer (void) const
{
Integer value (0);

if (!exists ()) {
return value;
}

if (type_ == INTEGER)
value = int_value_;
else if (type_ == INTEGER_ARRAY)
value = int_array_->size () == 0 ? 0 : int_array_->at(0);
else if (has_history())
return ref_newest ().to_integer ();
else if (type_ != EMPTY)
else if (has_history ())
value = ref_newest ().to_integer ();
else
{
std::stringstream buffer;

Expand All @@ -213,11 +225,11 @@ KnowledgeRecord::to_integers (void) const
{
std::vector <Integer> integers;

if (type_ == EMPTY) {
if (!exists ()) {
return integers;
}

if (has_history()) {
if (has_history ()) {
return ref_newest ().to_integers ();
}

Expand Down Expand Up @@ -267,11 +279,11 @@ KnowledgeRecord::to_doubles (void) const
{
std::vector <double> doubles;

if (type_ == EMPTY) {
if (!exists ()) {
return doubles;
}

if (has_history()) {
if (has_history ()) {
return ref_newest ().to_doubles ();
}

Expand Down Expand Up @@ -318,11 +330,11 @@ KnowledgeRecord::to_doubles (void) const
std::string
KnowledgeRecord::to_string (const std::string & delimiter) const
{
if (type_ == EMPTY) {
if (!exists ()) {
return "";
}

if (has_history()) {
if (has_history ()) {
return ref_newest ().to_string ();
}

Expand Down Expand Up @@ -480,7 +492,7 @@ KnowledgeRecord::to_unmanaged_buffer (size_t & size) const
size = sizeof(double) * double_array_->size () ;
buffer = new char [size];
memcpy (buffer, &(*double_array_)[0], size);
} else if (has_history ()) {
} else if (has_history () && !buf_->empty()) {
return ref_newest ().to_unmanaged_buffer (size);
} else {
buffer = nullptr;
Expand All @@ -494,12 +506,16 @@ KnowledgeRecord::to_unmanaged_buffer (size_t & size) const
KnowledgeRecord
KnowledgeRecord::fragment (unsigned int first, unsigned int last)
{
knowledge::KnowledgeRecord ret;

if (!exists ()) {
return ret;
}

if (has_history ()) {
return fragment (first, last);
}

knowledge::KnowledgeRecord ret;

if (is_string_type (type_))
{
unsigned int size = (unsigned int)str_value_->size ();
Expand Down Expand Up @@ -574,11 +590,13 @@ KnowledgeRecord::operator< (
const knowledge::KnowledgeRecord & rhs) const
{
if (has_history ()) {
return ref_newest ().operator< (rhs);
return (rhs.buf_->empty () ? KnowledgeRecord{} : ref_newest ()).
operator< (rhs);
}

if (rhs.has_history ()) {
return operator< (rhs.ref_newest ());
return operator< (rhs.buf_->empty () ?
KnowledgeRecord{} : rhs.ref_newest ());
}

Integer result (0);
Expand Down Expand Up @@ -668,11 +686,13 @@ KnowledgeRecord::operator<= (
const knowledge::KnowledgeRecord & rhs) const
{
if (has_history ()) {
return ref_newest ().operator<= (rhs);
return (rhs.buf_->empty () ? KnowledgeRecord{} : ref_newest ()).
operator<= (rhs);
}

if (rhs.has_history ()) {
return operator<= (rhs.ref_newest ());
return operator<= (rhs.buf_->empty () ?
KnowledgeRecord{} : rhs.ref_newest ());
}

Integer result (0);
Expand Down Expand Up @@ -762,11 +782,13 @@ KnowledgeRecord::operator== (
const knowledge::KnowledgeRecord & rhs) const
{
if (has_history ()) {
return ref_newest ().operator== (rhs);
return (rhs.buf_->empty () ? KnowledgeRecord{} : ref_newest ()).
operator== (rhs);
}

if (rhs.has_history ()) {
return operator== (rhs.ref_newest ());
return operator== (rhs.buf_->empty () ?
KnowledgeRecord{} : rhs.ref_newest ());
}

Integer result (0);
Expand Down Expand Up @@ -874,11 +896,13 @@ bool
KnowledgeRecord::operator> (const knowledge::KnowledgeRecord & rhs) const
{
if (has_history ()) {
return ref_newest ().operator> (rhs);
return (rhs.buf_->empty () ? KnowledgeRecord{} : ref_newest ()).
operator> (rhs);
}

if (rhs.has_history ()) {
return operator> (rhs.ref_newest ());
return operator> (rhs.buf_->empty () ?
KnowledgeRecord{} : rhs.ref_newest ());
}

Integer result (0);
Expand Down Expand Up @@ -969,11 +993,13 @@ bool
KnowledgeRecord::operator>= (const knowledge::KnowledgeRecord & rhs) const
{
if (has_history ()) {
return ref_newest ().operator>= (rhs);
return (rhs.buf_->empty () ? KnowledgeRecord{} : ref_newest ()).
operator>= (rhs);
}

if (rhs.has_history ()) {
return operator>= (rhs.ref_newest ());
return operator>= (rhs.buf_->empty () ?
KnowledgeRecord{} : rhs.ref_newest ());
}

Integer result (0);
Expand Down Expand Up @@ -1076,7 +1102,7 @@ KnowledgeRecord::retrieve_index (size_t index) const
if (index < size_t (double_array_-> size ()))
ret_value.set_value (double_array_->at (index));
}
else if (has_history ())
else if (has_history () && !buf_->empty ())
{
return ref_newest ().retrieve_index (index);
}
Expand Down Expand Up @@ -1170,7 +1196,7 @@ KnowledgeRecord::resize (size_t new_size)

if (new_size > cur_size)
{
if (type_ == EMPTY ||
if (!exists () ||
type_ == INTEGER)
{
Integer zero (0);
Expand Down Expand Up @@ -1291,7 +1317,7 @@ KnowledgeRecord::is_true (void) const
}
else if (has_history ())
{
return ref_newest ().is_true();
return !buf_->empty () && ref_newest ().is_true ();
}
else
{
Expand Down
44 changes: 23 additions & 21 deletions include/madara/knowledge/KnowledgeRecord.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1581,9 +1581,13 @@ namespace madara
{
if (type_ == BUFFER) {
return buf_->size();
} else {
}

if (exists()) {
return 1;
}

return 0;
}

/**
Expand All @@ -1609,7 +1613,7 @@ namespace madara
{
if (type_ == BUFFER) {
if (size <= 1) {
if (buf_->empty()) {
if (!buf_->empty()) {
*this = std::move(buf_->back());
} else {
reset_value();
Expand All @@ -1620,10 +1624,14 @@ namespace madara
} else {
if (size > 1) {
KnowledgeRecord tmp = *this;

new (&buf_) std::shared_ptr<CircBuf>(
std::make_shared<CircBuf>(size));
type_ = BUFFER;
buf_->push_back(std::move(tmp));

if (tmp.exists()) {
buf_->push_back(std::move(tmp));
}
}
}
}
Expand Down Expand Up @@ -1922,6 +1930,10 @@ namespace madara
return ret;
}

/**
* Gets the absolute index of the newest element in stored history.
* If this record doesn't have history capacity, returns 0.
**/
size_t get_history_newest_index() const
{
if (!has_history()) {
Expand All @@ -1930,6 +1942,10 @@ namespace madara
return buf_->back_index();
}

/**
* Gets the absolute index of the oldest element in stored history.
* If this record doesn't have history capacity, returns 0.
**/
size_t get_history_oldest_index() const
{
if (!has_history()) {
Expand All @@ -1938,24 +1954,10 @@ namespace madara
return buf_->front_index();
}

CircBuf::const_iterator begin_history() const
{
if (!has_history()) {
throw exceptions::MadaraException(
"begin_history() called on record without history");
}
return buf_->cbegin();
}

CircBuf::const_iterator end_history() const
{
if (!has_history()) {
throw exceptions::MadaraException(
"end_history() called on record without history");
}
return buf_->cend();
}

/**
* Get a shared_ptr to the history buffer inside this record. Returns
* nullptr if this record has no history capacity.
**/
std::shared_ptr<CircBuf> share_circular_buffer() const;

/**
Expand Down

0 comments on commit 581a9a8

Please sign in to comment.