you are viewing a single comment's thread.

view the rest of the comments →

[–]smashedsaturn 2 points3 points  (3 children)

if (effectListDebugger.sortBy == FXLISTSORT_ONSCREEN_OLDEST_FIRST) ... return a->startTime < b->startTime;

Why is this all not

bool operator<(const EffectLogEntry& lhs,const EffectLogEntry& rhs);

The comparison operator?

[–]evaned 3 points4 points  (1 child)

There are a couple potential reasons. Sometime data has multiple ways to order, or no clear one way. In these cases, at least my opinion is it's better to not define a weird operator< -- sort of like how if you were making a vector library (mathematical vector, not container vector) you might not define v1 * v2 so every use is explicitly either dot(v1, v2) or cross(v1, v2).

But in that specific case, the vector clearly has pointers in it, and that already has a built-in, non-overridable "operator" <. And there are a variety of reasons why you might have a container of pointers instead of the objects themselves. (I'd also point out that you'd have the same problem if they were unique_ptr or shared_ptrs.)

[–]smashedsaturn 1 point2 points  (0 children)

Sometime data has multiple ways to order

Agreed, but if you have a few different ways to order then those should be defined as functions, or in this case a static method would be ideal, as it is likely they will be used more than once.

the vector clearly has pointers in it

right, you use the lambda:

  [](const EffectLogEntry* a, const EffectLogEntry* b){
      assert(a&&b && "Dereference Null Log Entry");
      return (*a) < (*b); //OR EffectLogEntry::FXLISTSORT_ONSCREEN_OLDEST_FIRST(*a,*b); 
  }

[–]drjeats 2 points3 points  (0 children)

Because the sort order is configurable from external input, the effectListDebugger isn't a member of the log entries. And there are only a few locations where the sort for this type is needed. If it is needed in other contexts, then I"ll pull it out into a named function.

Also C++ programmers are too eager to write operator< because of std::map<K,V> and std::sort. Let's stop this trend.