MQL5 - CArray sorts only once

 

This isn't really a question per se. It's more of a comment on something that tripped me up for a bit. I found a work around, but maybe you can comment on a better work around.

So, CArray has a Sort() method that looks like this:

void CArray::Sort(const int mode)
  {
//--- check
   if(IsSorted(mode))
      return;
   m_sort_mode=mode;
   if(m_data_total<=1)
      return;
//--- sort
   QuickSort(0,m_data_total-1,mode);
  }

Note the IsSorted() call at the beginning. This means that repeated calls to this function with the same direction (mode) does not resort.

I have 28 nodes (CArrayObj) in which I update the volume every second, and I want to sort on the volume. This works fine the first time through, but because I don't add additional nodes or remove nodes or sort in another direction, it will not sort again.

My workaround was to override Sort() and remove the IsSorted() lines, which works.

Another possible workaround is to delete all the nodes then repopulate each time. I don't like that idea so much, but it means not overriding Sort().

I'm not sure I fully understand why it only sorts once per direction anyway.

Thoughts?

 
Anthony Garot Note the IsSorted() call at the beginning. This means that repeated calls to this function with the same direction (mode) does not resort.


I have 28 nodes (CArrayObj) in which I update the volume every second, and I want to sort on the volume. This works fine the first time through, but because I don't add additional nodes or remove nodes or sort in another direction, it will not sort again.

No work around is needed.

It doesn't sort again because the array is already in the proper order and you haven't added or inserted new values or modified existing. Nothing has changed since the last sort.

If you add or insert new value(s) or update the existing values, the routine invalidates the sort mode and it will resort.

bool CArrayDouble::Add(const double element)
  {
//--- check/reserve elements of array
   if(!Reserve(1))
      return(false);
//--- add
   m_data[m_data_total++]=element;
   m_sort_mode=-1;
//--- successful
   return(true);
  }

bool CArrayObj::Update(const int index,CObject *element)
  {
//--- check
   if(index<0 || !CheckPointer(element) || index>=m_data_total)
      return(false);
//--- "physical" removal of the object (if necessary and possible)
   if(m_free_mode && CheckPointer(m_data[index])==POINTER_DYNAMIC)
      delete m_data[index];
//--- update
   m_data[index]=element;
   m_sort_mode=-1;
//--- successful
   return(true);
  }

You are modifying existing objects but not telling the array.

  1. Turn off free mode
  2. For each entry, get the node, modify it, update the array with it.
  3. Turn on free mode
  4. sort.
 
whroeder1:
  1. Turn off free mode
  2. For each entry, get the node, modify it, update the array with it.
  3. Turn on free mode
  4. sort.

Yup. That works, and it's pure. Thanks!

 

You can also subclass CarrayObj...

#include <trade/trade.mqh>
#include <arrays/arrayobj.mqh>

class MySymbol : public CSymbolInfo {
public:
   MySymbol(string symbol) { this.Name(symbol); }
   virtual int Compare(const CObject *node, const int mode=0) const override {
      const MySymbol *other = node;
      if (this.Spread() > other.Spread())
         return 1;
      if (this.Spread() < other.Spread())
         return -1;
      return 0;
   }
};

template <typename T>
class objvector : public CArrayObj {
public:
   T operator[](const int i) const { 
      return this.At(i); 
   }
   void Sort(const int mode=0) {
      this.m_sort_mode = -1;
      CArrayObj::Sort(mode);
   }
};

void OnStart() {
   objvector<MySymbol*> symbols;
   for (int i=SymbolsTotal(true)-1; i>=0; --i) {
      symbols.Add(new MySymbol(SymbolName(i, true)));
   }
   while (!IsStopped()) {
      symbols.Sort();
   }
}
 
nicholi shen:

You can also subclass CarrayObj...

Actually, your solution above is about 99% of my final solution. I borrowed your concept of a sortable vector from a post of yours a long time ago. Thank you for that.

The only difference of my code now is that I added a method AlwaysSort() to objVector—instead of overriding Sort(). That way, I know at a glance that I have deviated from the original Sort() method.

template<typename T>
class CObjVector : public CArrayObj
{
public:
    T *operator[](const int index) const { return (T*)At(index);}

    // Can't use Sort directly because it only sorts once.
    void AlwaysSort(const int mode)
    {
       m_sort_mode=mode;
       if(m_data_total<=1)
          return;
        //--- sort
       QuickSort(0,m_data_total-1,mode);
    }
};

I surely appreciate @whroeder1's input, because it showed me what the original writers of the class were thinking. What I didn't like about using a pure solution is that i needed to add three extra steps that arguably obfuscated what the code was doing.

// This is how to use .Sort() without my .AlwaysSort() function.
oVolume[column].FreeMode(false);        // Will not delete the object
oVolume[column].Update(j,node);         // Update the object with itself
oVolume[column].FreeMode(true);         // Will delete the object
 

IMHO, the original developers weren't really considering object sorting based on mutable object properties/methods, and were just keeping a consistent interface across the CArray subclasses. That's why I like to override the Sort method for CArrayObj subclasses -- because unlike CArray*PrimativeType, I want to re-sort the object vector every time. I really like your idea of having a separate explicit method for sort always, but unfortunately for me it'd require copious amounts of refactoring. Also, you only need to change m_sort_mode value and then call back to the original Sort method. 

    void AlwaysSort(const int mode)
    {
       m_sort_mode = WRONG_VALUE;
       CArrayObj::Sort(mode);
    }
 
nicholi shen:

Yeah, your snippet is cleaner and better illustrates the point. Thanks, I will use it.

 
Anthony Garot:

Yeah, your snippet is cleaner and better illustrates the point. Thanks, I will use it.

Cool. One more suggestion... You may want to alter your operator overload so that you have to explicitly define that the collection returns pointers in the instance declaration. Early on I did it the way you are doing it, but the syntax is inconsistent with how you would declare a vector of pointers in C.
template<typename T>
class CObjVector : public CArrayObj {
public:
    T operator[](const int index) const { return this.At(index); }
};

CObjVector<CObject*> objects;

...
 
nicholi shen:
Cool. One more suggestion... You may want to alter your operator overload so that you have to explicitly define that the collection returns pointers in the instance declaration. Early on I did it the way you are doing it, but the syntax is inconsistent with how you would declare a vector of pointers in C.
Yup. Makes total sense. Updated my code.