Generic Class Library - bugs, description, questions, usage and suggestions - page 20

 
Sergey Dzyublik:

1. Eliminate ambiguous behavior:
If you pass "INT_MAX - 10" as a parameter into CPrimeGenerator::ExpandPrime, the result "INT_MAX" will be returned.
If "INT_MAX - 10" is passed as a parameter in CPrimeGenerator::GetPrime, the same result will be returned: "INT_MAX - 10".

Also in both cases the returned value is not a prime number which misleads the user.

On the first point:

There is no ambiguity here.

The GetPrime method should return the closest prime number from above, but on the interval from INT_MAX - 10 to INT_MAX there are none, therefore INT_MAX - 10 is returned.

The ExpandPrime method first doubles the input value and then the GetPrime method is called from the received number.

In addition, ExpandPrime has a check for INT_MAX overrun:

   if((uint)new_size>INT_MAX && INT_MAX>old_size)
      return INT_MAX;
   else
      return GetPrime(new_size);

In my opinion, the logic of behavior of these methods is absolutely unambiguous and correct.


Concerning the second and third points:

The changes you suggest are only relevant for the most part to edge problems, when CHashMap sizes are very large. However, there is no guarantee that they will have a positive impact on performance, so we need to conduct separate research to determine the correctness of the changes you propose.

 
Roman Konopelko:

There is no ambiguity here.
The GetPrime method should return the closest prime number from above, but on the interval from INT_MAX - 10 to INT_MAX there are none, that is why INT_MAX - 10 is returned.
The ExpandPrime method first doubles the input value and then the GetPrime method is called from the received number.
In addition, ExpandPrime has a check for INT_MAX overrun:
In my opinion, the logic of behavior of these methods is absolutely unambiguous and correct.


1. The functions may not return prime numbers, but unknown numbers.
How the user uses this data is his problem, maybe he casts it into the long and passes it on to the supercomputing functions, who cares.
The fact is that it may not return what is declared and expected by default from the functions.

2. How do you check that a function call returned a prime number and not something else?
You cannot simply compare it to INT_MAX.
You need to compare it to the last available prime number less than INT_MAX.
To compare the result of each call of these functions with some magic number each time to make sure everything works right - sounds like nonsense to me.

 
Sergey Dzyublik:

1. Functions may not return prime numbers, but obscure numbers.
How the user uses this data is his problem, maybe he casts it to long and then passes it to supercomputing functions, who cares.
The fact is that it may not return what is declared and expected by default from the functions.

2. How do you check that a function call returned a prime number and not something else?
You cannot simply compare it to INT_MAX.
You need to compare it to the last available prime number less than INT_MAX.
Each time to compare the result of these functions' calls with some magic number to make sure everything works properly - sounds like nonsense to me.

1. Your case of getting a non-simple number using the GetPrime method is the only one I've encountered so far. This incident will be fixed by changing the check when generating prime numbers:

//--- outside of our predefined table
   for(int i=(min|1); i<=INT_MAX; i+=2)
     {
      if(IsPrime(i) && ((i-1)%s_hash_prime!=0))
         return(i);
     }
   return(min);

2. CPrimeGenerator::IsPrime method to check the number for simplicity

 

I tried to switch from my ArrayList to yours, which is in Generic/ArrayList.mqh

ME gives nothing after ".".

How do I get the value? Get() and [] are missing from the class.

And it doesn't take into account that there might be an array of pointers.

And who creates this library?

Here's my version of ArrayList from Java:

Files:
ArrayList.mqh  46 kb
 
There were no class templates when I created this variant.
 
This isthe only way to make the terminal calculate the hash codes automatically, for example by means of .Net in MQL5:

For Generic collections to work correctly with class objects, these classes must implement IEqualityComparable interface that has Equals and HashCode methods defined. That is, the user must set methods of calculating hash codes by himself, and this is the only option so far, because it's impossible to implement these methods automatically, as it was done in .Net, for example, by means of MQL5.

Then why does your template work with any types, misleading the programmer? If only classes inherited from IEqualityComparable work correctly, it means that it is necessary to prohibit work with other types at compiler level.

Let me remind you of this code:

//+------------------------------------------------------------------+
//| Returns a hashcode for custom object.                            |
//+------------------------------------------------------------------+
template<typename T>
int GetHashCode(T value)
  {
//--- try to convert to equality comparable object  
   IEqualityComparable<T>*equtable=dynamic_cast<IEqualityComparable<T>*>(value);
   if(equtable)
     {
      //--- calculate hash by specied method   
      return equtable.HashCode();
     }
   else
     {
      //--- calculate hash from name of object
      return GetHashCode(typename(value));
     }
  }

I think this function should be replaced with this one:

template<typename  T>
int GetHashCode(IEqualityComparable<T> &value)
  {
    return value.HashCode()
  }
 
Alexey Navoykov:

Then why does your template work with any types, misleading the programmer? If only classes inherited from IEqualityComparable work correctly, then you should disable working with other types at compiler level.

It will be uncomfortable to work with it. The GetHashCode overloads for the standard types show the interface for getting hash code.


Absence of it is disturbing.

template<typename T>
interface IEqualityComparable
  {
//--- method for determining equality
   bool              Equals(T & value);
//--- method to calculate hash code   
   int               HashCode(void);
  };


I.e. for objects bummer now.

 
fxsaber:

It would be uncomfortable to work with such. The GetHashCode overloads for the standard types show the interface for getting hash code.

And what's convenient now? The fact that passing an enum or a pointer of a class that doesn't support the interface into this function, you just get the class name? ) And most importantly, the code works and compiles as if everything is normal.

 
Alexey Navoykov:

What's the convenience now? That passing an enum or pointer of a class that doesn't support the interface into this function, you just get the name of the class? Awesome hash ) And most importantly, the code works, compiles as if everything is normal. This is not the case.

Yes, they made a bullshit. thoughtlessly copied from the NetFramework. Although it's obvious that without support for interfaces at the language level, it can not work adequately. I remember the level of MQ codes 6-7 years ago and now.

 
Alexey Navoykov:

What's convenient now? The fact that passing an enum or a pointer of a class that doesn't support the interface into this function, you just get the class name? ) And most importantly, the code works and compiles as if everything is normal. This is not the case.

I agree, it's better to get a compilation error right away than deal with why it doesn't work.

Honestly speaking, I don't even know why they put the interface there. All you need is to overload GetHashCode for the required type and not to start making IEqualityComparable.

Reason: