Task: Remove std::map inheritance, clean up interface

- Make the Task object's interface more explicit by removing the
  std::map inheritance.
- Using this more explicit interface, remove unneeded ctors in order to
  allow the compiler to "Do The Right Thing"(tm).
  This leads to a performance improvement of 12% in the "add"
  performance test, and 7% for "import".
This commit is contained in:
Wilhelm Schuermann 2015-11-04 20:33:52 +01:00
parent c248a32cab
commit 8f8ad813cd
12 changed files with 64 additions and 92 deletions

View file

@ -211,13 +211,13 @@ bool DOM::get (const std::string& name, const Task& task, Variant& value)
return false; return false;
// Quickly deal with the most common cases. // Quickly deal with the most common cases.
if (task.size () && name == "id") if (task.data.size () && name == "id")
{ {
value = Variant (static_cast<int> (task.id)); value = Variant (static_cast<int> (task.id));
return true; return true;
} }
if (task.size () && name == "urgency") if (task.data.size () && name == "urgency")
{ {
value = Variant (task.urgency_c ()); value = Variant (task.urgency_c ());
return true; return true;
@ -262,13 +262,13 @@ bool DOM::get (const std::string& name, const Task& task, Variant& value)
{ {
// Now that 'ref' is the contextual task, and any ID/UUID is chopped off the // Now that 'ref' is the contextual task, and any ID/UUID is chopped off the
// elements vector, DOM resolution is now simple. // elements vector, DOM resolution is now simple.
if (ref.size () && size == 1 && canonical == "id") if (ref.data.size () && size == 1 && canonical == "id")
{ {
value = Variant (static_cast<int> (ref.id)); value = Variant (static_cast<int> (ref.id));
return true; return true;
} }
if (ref.size () && size == 1 && canonical == "urgency") if (ref.data.size () && size == 1 && canonical == "urgency")
{ {
value = Variant (ref.urgency_c ()); value = Variant (ref.urgency_c ());
return true; return true;
@ -276,7 +276,7 @@ bool DOM::get (const std::string& name, const Task& task, Variant& value)
Column* column = context.columns[canonical]; Column* column = context.columns[canonical];
if (ref.size () && size == 1 && column) if (ref.data.size () && size == 1 && column)
{ {
if (column->is_uda () && ! ref.has (canonical)) if (column->is_uda () && ! ref.has (canonical))
{ {
@ -311,13 +311,13 @@ bool DOM::get (const std::string& name, const Task& task, Variant& value)
return true; return true;
} }
if (ref.size () && size == 2 && canonical == "tags") if (ref.data.size () && size == 2 && canonical == "tags")
{ {
value = Variant (ref.hasTag (elements[1]) ? elements[1] : ""); value = Variant (ref.hasTag (elements[1]) ? elements[1] : "");
return true; return true;
} }
if (ref.size () && size == 2 && column && column->type () == "date") if (ref.data.size () && size == 2 && column && column->type () == "date")
{ {
ISO8601d date (ref.get_date (canonical)); ISO8601d date (ref.get_date (canonical));
if (elements[1] == "year") { value = Variant (static_cast<int> (date.year ())); return true; } if (elements[1] == "year") { value = Variant (static_cast<int> (date.year ())); return true; }
@ -332,7 +332,7 @@ bool DOM::get (const std::string& name, const Task& task, Variant& value)
} }
} }
if (ref.size () && size == 3 && elements[0] == "annotations") if (ref.data.size () && size == 3 && elements[0] == "annotations")
{ {
std::map <std::string, std::string> annos; std::map <std::string, std::string> annos;
ref.getAnnotations (annos); ref.getAnnotations (annos);
@ -361,7 +361,7 @@ bool DOM::get (const std::string& name, const Task& task, Variant& value)
} }
} }
if (ref.size () && size == 4 && elements[0] == "annotations" && elements[2] == "entry") if (ref.data.size () && size == 4 && elements[0] == "annotations" && elements[2] == "entry")
{ {
std::map <std::string, std::string> annos; std::map <std::string, std::string> annos;
ref.getAnnotations (annos); ref.getAnnotations (annos);

View file

@ -990,11 +990,11 @@ void TDB2::show_diff (
Task before (prior); Task before (prior);
std::vector <std::string> beforeAtts; std::vector <std::string> beforeAtts;
for (auto& att : before) for (auto& att : before.data)
beforeAtts.push_back (att.first); beforeAtts.push_back (att.first);
std::vector <std::string> afterAtts; std::vector <std::string> afterAtts;
for (auto& att : after) for (auto& att : after.data)
afterAtts.push_back (att.first); afterAtts.push_back (att.first);
std::vector <std::string> beforeOnly; std::vector <std::string> beforeOnly;
@ -1009,7 +1009,7 @@ void TDB2::show_diff (
view.set (row, 1, renderAttribute (name, before.get (name)), color_red); view.set (row, 1, renderAttribute (name, before.get (name)), color_red);
} }
for (auto& att : before) for (auto& att : before.data)
{ {
std::string priorValue = before.get (att.first); std::string priorValue = before.get (att.first);
std::string currentValue = after.get (att.first); std::string currentValue = after.get (att.first);
@ -1035,7 +1035,7 @@ void TDB2::show_diff (
else else
{ {
int row; int row;
for (auto& att : after) for (auto& att : after.data)
{ {
row = view.addRow (); row = view.addRow ();
view.set (row, 0, att.first); view.set (row, 0, att.first);
@ -1093,11 +1093,11 @@ void TDB2::show_diff (
std::vector <std::string> all = context.getColumns (); std::vector <std::string> all = context.getColumns ();
// Now factor in the annotation attributes. // Now factor in the annotation attributes.
for (auto& it : before) for (auto& it : before.data)
if (it.first.substr (0, 11) == "annotation_") if (it.first.substr (0, 11) == "annotation_")
all.push_back (it.first); all.push_back (it.first);
for (auto& it : after) for (auto& it : after.data)
if (it.first.substr (0, 11) == "annotation_") if (it.first.substr (0, 11) == "annotation_")
all.push_back (it.first); all.push_back (it.first);

View file

@ -90,7 +90,8 @@ static const std::string dummy ("");
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
Task::Task () Task::Task ()
: id (0) : data ()
, id (0)
, urgency_value (0.0) , urgency_value (0.0)
, recalc_urgency (true) , recalc_urgency (true)
, is_blocked (false) , is_blocked (false)
@ -99,29 +100,6 @@ Task::Task ()
{ {
} }
////////////////////////////////////////////////////////////////////////////////
Task::Task (const Task& other)
{
*this = other;
}
////////////////////////////////////////////////////////////////////////////////
Task& Task::operator= (const Task& other)
{
if (this != &other)
{
std::map <std::string, std::string>::operator= (other);
id = other.id;
urgency_value = other.urgency_value;
recalc_urgency = other.recalc_urgency;
is_blocked = other.is_blocked;
is_blocking = other.is_blocking;
annotation_count = other.annotation_count;
}
return *this;
}
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// The uuid and id attributes must be exempt from comparison. // The uuid and id attributes must be exempt from comparison.
// //
@ -136,10 +114,10 @@ Task& Task::operator= (const Task& other)
// set sizes. // set sizes.
bool Task::operator== (const Task& other) bool Task::operator== (const Task& other)
{ {
if (size () != other.size ()) if (data.size () != other.data.size ())
return false; return false;
for (auto& i : *this) for (auto& i : data)
if (i.first != "uuid" && if (i.first != "uuid" &&
i.second != other.get (i.first)) i.second != other.get (i.first))
return false; return false;
@ -173,11 +151,6 @@ Task::Task (const json::object* obj)
parseJSON (obj); parseJSON (obj);
} }
////////////////////////////////////////////////////////////////////////////////
Task::~Task ()
{
}
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
Task::status Task::textToStatus (const std::string& input) Task::status Task::textToStatus (const std::string& input)
{ {
@ -228,7 +201,7 @@ void Task::setAsNow (const std::string& att)
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
bool Task::has (const std::string& name) const bool Task::has (const std::string& name) const
{ {
if (find (name) != end ()) if (data.find (name) != data.end ())
return true; return true;
return false; return false;
@ -238,7 +211,7 @@ bool Task::has (const std::string& name) const
std::vector <std::string> Task::all () std::vector <std::string> Task::all ()
{ {
std::vector <std::string> all; std::vector <std::string> all;
for (auto i : *this) for (auto i : data)
all.push_back (i.first); all.push_back (i.first);
return all; return all;
@ -247,8 +220,8 @@ std::vector <std::string> Task::all ()
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
const std::string Task::get (const std::string& name) const const std::string Task::get (const std::string& name) const
{ {
auto i = find (name); auto i = data.find (name);
if (i != end ()) if (i != data.end ())
return i->second; return i->second;
return ""; return "";
@ -257,8 +230,8 @@ const std::string Task::get (const std::string& name) const
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
const std::string& Task::get_ref (const std::string& name) const const std::string& Task::get_ref (const std::string& name) const
{ {
auto i = find (name); auto i = data.find (name);
if (i != end ()) if (i != data.end ())
return i->second; return i->second;
return dummy; return dummy;
@ -267,8 +240,8 @@ const std::string& Task::get_ref (const std::string& name) const
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
int Task::get_int (const std::string& name) const int Task::get_int (const std::string& name) const
{ {
auto i = find (name); auto i = data.find (name);
if (i != end ()) if (i != data.end ())
return strtol (i->second.c_str (), NULL, 10); return strtol (i->second.c_str (), NULL, 10);
return 0; return 0;
@ -277,8 +250,8 @@ int Task::get_int (const std::string& name) const
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
unsigned long Task::get_ulong (const std::string& name) const unsigned long Task::get_ulong (const std::string& name) const
{ {
auto i = find (name); auto i = data.find (name);
if (i != end ()) if (i != data.end ())
return strtoul (i->second.c_str (), NULL, 10); return strtoul (i->second.c_str (), NULL, 10);
return 0; return 0;
@ -287,8 +260,8 @@ unsigned long Task::get_ulong (const std::string& name) const
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
float Task::get_float (const std::string& name) const float Task::get_float (const std::string& name) const
{ {
auto i = find (name); auto i = data.find (name);
if (i != end ()) if (i != data.end ())
return strtof (i->second.c_str (), NULL); return strtof (i->second.c_str (), NULL);
return 0.0; return 0.0;
@ -297,8 +270,8 @@ float Task::get_float (const std::string& name) const
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
time_t Task::get_date (const std::string& name) const time_t Task::get_date (const std::string& name) const
{ {
auto i = find (name); auto i = data.find (name);
if (i != end ()) if (i != data.end ())
return (time_t) strtoul (i->second.c_str (), NULL, 10); return (time_t) strtoul (i->second.c_str (), NULL, 10);
return 0; return 0;
@ -307,7 +280,7 @@ time_t Task::get_date (const std::string& name) const
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
void Task::set (const std::string& name, const std::string& value) void Task::set (const std::string& name, const std::string& value)
{ {
(*this)[name] = json::decode (value); data[name] = json::decode (value);
recalc_urgency = true; recalc_urgency = true;
} }
@ -315,7 +288,7 @@ void Task::set (const std::string& name, const std::string& value)
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
void Task::set (const std::string& name, int value) void Task::set (const std::string& name, int value)
{ {
(*this)[name] = format (value); data[name] = format (value);
recalc_urgency = true; recalc_urgency = true;
} }
@ -323,7 +296,7 @@ void Task::set (const std::string& name, int value)
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
void Task::remove (const std::string& name) void Task::remove (const std::string& name)
{ {
if (erase (name)) if (data.erase (name))
recalc_urgency = true; recalc_urgency = true;
} }
@ -539,7 +512,7 @@ bool Task::is_udaPresent () const
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
bool Task::is_orphanPresent () const bool Task::is_orphanPresent () const
{ {
for (auto& att : *this) for (auto& att : data)
if (att.first.compare (0, 11, "annotation_", 11) != 0) if (att.first.compare (0, 11, "annotation_", 11) != 0)
if (context.columns.find (att.first) == context.columns.end ()) if (context.columns.find (att.first) == context.columns.end ())
return true; return true;
@ -586,7 +559,7 @@ void Task::parse (const std::string& input)
{ {
// File format version 4, from 2009-5-16 - now, v1.7.1+ // File format version 4, from 2009-5-16 - now, v1.7.1+
// This is the parse format tried first, because it is most used. // This is the parse format tried first, because it is most used.
clear (); data.clear ();
if (input[0] == '[') if (input[0] == '[')
{ {
@ -614,7 +587,7 @@ void Task::parse (const std::string& input)
if (! name.compare (0, 11, "annotation_", 11)) if (! name.compare (0, 11, "annotation_", 11))
++annotation_count; ++annotation_count;
(*this)[name] = decode (json::decode (value)); data[name] = decode (json::decode (value));
} }
nl.skip (' '); nl.skip (' ');
@ -814,7 +787,7 @@ std::string Task::composeF4 () const
std::string ff4 = "["; std::string ff4 = "[";
bool first = true; bool first = true;
for (auto it : *this) for (auto it : data)
{ {
std::string type = Task::attributes[it.first]; std::string type = Task::attributes[it.first];
if (type == "") if (type == "")
@ -852,7 +825,7 @@ std::string Task::composeJSON (bool decorate /*= false*/) const
// First the non-annotations. // First the non-annotations.
int attributes_written = 0; int attributes_written = 0;
for (auto& i : *this) for (auto& i : data)
{ {
// Annotations are not written out here. // Annotations are not written out here.
if (! i.first.compare (0, 11, "annotation_", 11)) if (! i.first.compare (0, 11, "annotation_", 11))
@ -962,7 +935,7 @@ std::string Task::composeJSON (bool decorate /*= false*/) const
<< "\"annotations\":["; << "\"annotations\":[";
int annotations_written = 0; int annotations_written = 0;
for (auto& i : *this) for (auto& i : data)
{ {
if (! i.first.compare (0, 11, "annotation_", 11)) if (! i.first.compare (0, 11, "annotation_", 11))
{ {
@ -1019,7 +992,7 @@ void Task::addAnnotation (const std::string& description)
} }
while (has (key)); while (has (key));
(*this)[key] = json::decode (description); data[key] = json::decode (description);
++annotation_count; ++annotation_count;
recalc_urgency = true; recalc_urgency = true;
} }
@ -1028,13 +1001,13 @@ void Task::addAnnotation (const std::string& description)
void Task::removeAnnotations () void Task::removeAnnotations ()
{ {
// Erase old annotations. // Erase old annotations.
auto i = begin (); auto i = data.begin ();
while (i != end ()) while (i != data.end ())
{ {
if (! i->first.compare (0, 11, "annotation_", 11)) if (! i->first.compare (0, 11, "annotation_", 11))
{ {
--annotation_count; --annotation_count;
erase (i++); data.erase (i++);
} }
else else
i++; i++;
@ -1048,7 +1021,7 @@ void Task::getAnnotations (std::map <std::string, std::string>& annotations) con
{ {
annotations.clear (); annotations.clear ();
for (auto& ann : *this) for (auto& ann : data)
if (! ann.first.compare (0, 11, "annotation_", 11)) if (! ann.first.compare (0, 11, "annotation_", 11))
annotations.insert (ann); annotations.insert (ann);
} }
@ -1060,7 +1033,7 @@ void Task::setAnnotations (const std::map <std::string, std::string>& annotation
removeAnnotations (); removeAnnotations ();
for (auto& anno : annotations) for (auto& anno : annotations)
insert (anno); data.insert (anno);
annotation_count = annotations.size (); annotation_count = annotations.size ();
recalc_urgency = true; recalc_urgency = true;
@ -1291,7 +1264,7 @@ void Task::removeTag (const std::string& tag)
// A UDA Orphan is an attribute that is not represented in context.columns. // A UDA Orphan is an attribute that is not represented in context.columns.
void Task::getUDAOrphans (std::vector <std::string>& names) const void Task::getUDAOrphans (std::vector <std::string>& names) const
{ {
for (auto& it : *this) for (auto& it : data)
if (it.first.compare (0, 11, "annotation_", 11) != 0) if (it.first.compare (0, 11, "annotation_", 11) != 0)
if (context.columns.find (it.first) == context.columns.end ()) if (context.columns.find (it.first) == context.columns.end ())
names.push_back (it.first); names.push_back (it.first);

View file

@ -34,7 +34,7 @@
#include <time.h> #include <time.h>
#include <JSON.h> #include <JSON.h>
class Task : public std::map <std::string, std::string> class Task
{ {
public: public:
static std::string defaultProject; static std::string defaultProject;
@ -58,12 +58,11 @@ public:
public: public:
Task (); // Default constructor Task (); // Default constructor
Task (const Task&); // Copy constructor
Task& operator= (const Task&); // Assignment operator
bool operator== (const Task&); // Comparison operator bool operator== (const Task&); // Comparison operator
Task (const std::string&); // Parse Task (const std::string&); // Parse
Task (const json::object*); // Parse Task (const json::object*); // Parse
~Task (); // Destructor
std::map <std::string, std::string> data;
void parse (const std::string&); void parse (const std::string&);
std::string composeF4 () const; std::string composeF4 () const;

View file

@ -124,7 +124,7 @@ int CmdDenotate::execute (std::string&)
} }
} }
if (before != task) if (before.data != task.data)
{ {
std::string question = format (STRING_CMD_DENO_CONFIRM, std::string question = format (STRING_CMD_DENO_CONFIRM,
task.identifier (true), task.identifier (true),

View file

@ -205,7 +205,7 @@ void CmdImport::importSingleTask (json::object* obj)
if (hasGeneratedEnd) if (hasGeneratedEnd)
task.set ("end", before.get ("end")); task.set ("end", before.get ("end"));
if (before != task) if (before.data != task.data)
{ {
CmdModify modHelper; CmdModify modHelper;
modHelper.checkConsistency (before, task); modHelper.checkConsistency (before, task);

View file

@ -76,7 +76,7 @@ int CmdModify::execute (std::string&)
Task before (task); Task before (task);
task.modify (Task::modReplace); task.modify (Task::modReplace);
if (before != task) if (before.data != task.data)
{ {
// Abort if change introduces inconsistencies. // Abort if change introduces inconsistencies.
checkConsistency(before, task); checkConsistency(before, task);

View file

@ -137,7 +137,7 @@ int CmdUDAs::execute (std::string& output)
std::map <std::string, int> orphans; std::map <std::string, int> orphans;
for (auto& i : filtered) for (auto& i : filtered)
{ {
for (auto& att : i) for (auto& att : i.data)
if (att.first.substr (0, 11) != "annotation_" && if (att.first.substr (0, 11) != "annotation_" &&
context.columns.find (att.first) == context.columns.end ()) context.columns.find (att.first) == context.columns.end ())
orphans[att.first]++; orphans[att.first]++;

View file

@ -50,11 +50,11 @@ std::string taskDifferences (const Task& before, const Task& after)
// Attributes are all there is, so figure the different attribute names // Attributes are all there is, so figure the different attribute names
// between before and after. // between before and after.
std::vector <std::string> beforeAtts; std::vector <std::string> beforeAtts;
for (auto& att : before) for (auto& att : before.data)
beforeAtts.push_back (att.first); beforeAtts.push_back (att.first);
std::vector <std::string> afterAtts; std::vector <std::string> afterAtts;
for (auto& att : after) for (auto& att : after.data)
afterAtts.push_back (att.first); afterAtts.push_back (att.first);
std::vector <std::string> beforeOnly; std::vector <std::string> beforeOnly;
@ -144,11 +144,11 @@ std::string taskInfoDifferences (
// Attributes are all there is, so figure the different attribute names // Attributes are all there is, so figure the different attribute names
// between before and after. // between before and after.
std::vector <std::string> beforeAtts; std::vector <std::string> beforeAtts;
for (auto& att : before) for (auto& att : before.data)
beforeAtts.push_back (att.first); beforeAtts.push_back (att.first);
std::vector <std::string> afterAtts; std::vector <std::string> afterAtts;
for (auto& att : after) for (auto& att : after.data)
afterAtts.push_back (att.first); afterAtts.push_back (att.first);
std::vector <std::string> beforeOnly; std::vector <std::string> beforeOnly;

View file

@ -192,7 +192,7 @@ static void colorizeKeyword (Task& task, const std::string& rule, const Color& b
// first match. // first match.
else else
{ {
for (auto& it : task) for (auto& it : task.data)
{ {
if (! it.first.compare (0, 11, "annotation_", 11) && if (! it.first.compare (0, 11, "annotation_", 11) &&
find (it.second, rule.substr (14), sensitive) != std::string::npos) find (it.second, rule.substr (14), sensitive) != std::string::npos)

View file

@ -189,7 +189,7 @@ TODO Task::decode
test.is (task.get ("three"), "four", "three=four"); test.is (task.get ("three"), "four", "three=four");
// Task::set // Task::set
task.clear (); task.data.clear ();
task.set ("name", "value"); task.set ("name", "value");
test.is (task.composeF4 (), "[name:\"value\"]", "Task::set"); test.is (task.composeF4 (), "[name:\"value\"]", "Task::set");
@ -213,7 +213,7 @@ TODO Task::decode
test.is (task.composeF4 (), "[name:\"value\"]", "Task::remove"); test.is (task.composeF4 (), "[name:\"value\"]", "Task::remove");
// Task::all // Task::all
test.is (task.size (), (size_t)1, "Task::all size"); test.is (task.data.size (), (size_t)1, "Task::all size");
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////

View file

@ -78,7 +78,7 @@ int main (int, char**)
Task rightAgain (right); Task rightAgain (right);
std::string output = taskDifferences (left, right); std::string output = taskDifferences (left, right);
t.ok (left != right, "Detected changes"); t.ok (left.data != right.data, "Detected changes");
t.ok (output.find ("Zero will be changed from '0' to '00'") != std::string::npos, "Detected change zero:0 -> zero:00"); t.ok (output.find ("Zero will be changed from '0' to '00'") != std::string::npos, "Detected change zero:0 -> zero:00");
t.ok (output.find ("One will be deleted") != std::string::npos, "Detected deletion one:1 ->"); t.ok (output.find ("One will be deleted") != std::string::npos, "Detected deletion one:1 ->");
t.ok (output.find ("Two") == std::string::npos, "Detected no change two:2 -> two:2"); t.ok (output.find ("Two") == std::string::npos, "Detected no change two:2 -> two:2");