Wait until tomorrow pls, it has a bug, I will correct tomorrow, thx

Ok, sorry about that, I wrote that a bit quick, I introduced the opposite issue :-). Here is the version I propose, it works with non-array objects, primitive and object arrays:

public static String valueToString(final Object value) {

  if (value == null) {
    return "<null>";
  }

  if(value.getClass().isArray()) {

    if(Array.getLength(value) == 0) {
      return "";
    }

    String varr = null;
    char t = DataTypes.getTypeId(value);
    switch (t) {
    case 'l' :
      varr = value instanceof long[] ? Arrays.toString((long[]) value) : Arrays.toString((Long[]) value);
      break;

    case 'i' :
      varr = value instanceof int[] ? Arrays.toString((int[]) value) : Arrays.toString((Integer[]) value);
      break;

    case 'b' :
      varr = value instanceof boolean[] ? Arrays.toString((boolean[]) value) : Arrays.toString((Boolean[]) value);
      break;

    case 'f' :
      varr = value instanceof float[] ? Arrays.toString((float[]) value) : Arrays.toString((Float[]) value);
      break;

    case 'd' :
      varr = value instanceof double[] ? Arrays.toString((double[]) value) : Arrays.toString((Double[]) value);
      break;

    default :
      varr = Arrays.toString((Object[]) value);
    }
    return varr.substring(1, varr.length() - 1);
  }

  return value.toString();
}

Thank you for your bug report. Unfortunately I do not understand the problem.

Why do you think there is a bug in the Tigase code? What part of the Tigase code does not work correctly and how the problem manifest itself? The method you mentioning has been working for years and we have never seen any problems with the current implementation.

Artur Hefczyc wrote:

Thank you for your bug report. Unfortunately I do not understand the problem.

Why do you think there is a bug in the Tigase code? What part of the Tigase code does not work correctly and how the problem manifest itself? The method you mentioning has been working for years and we have never seen any problems with the current implementation.

Hi Artur,

If you define a property that is an array of a type that has both a primitive and an object representation (e.g. int[] / Integer[]) and you use the object variant instead of the primitive variant (e.g. Integer[]) then tigase.util.DataTypes.valueToString(Object) as implemented in Tigase throws a @ClassCastException@.

I believe it should not for, consistancy, and you should be able to use either the object or primitive types for arrays, just as you can for non-array types (boolean / Boolean; see examples here: http://docs.tigase.org/tigase-server/5.2.2/dev_guide/html/#cil2). To illustrate:

import java.util.Map;
import java.util.logging.Logger;
import tigase.server.AbstractMessageReceiver;
import tigase.server.Packet;

public class TestComponent extends AbstractMessageReceiver {

  private static final Logger log =
    Logger.getLogger(TestComponent.class.getName());

  private static final String COUNTERS1_KEY = "counters1";
  private static final String COUNTERS2_KEY = "counters2";

  private int[] counters1 = {1, 2, 3};
  private Integer[] counters2 = {1, 2, 3};

  @Override
  public void processPacket(Packet packet) {
    for (String pType : packetTypes) {
      if (pType == packet.getElemName()) {
        log.finest(prependText + packet.toString(secureLogging));
      }
    }
  }

  @Override
  public Map<String, Object> getDefaults(Map<String, Object> params) {
    Map<String, Object> defs = super.getDefaults(params);
    defs.put(COUNTERS1_KEY, counters1);
    defs.put(COUNTERS2_KEY, counters2);
    return defs;
  }

  @Override
  public void setProperties(Map<String, Object> props) {
    super.setProperties(props);
    if (props.get(COUNTERS1_KEY) != null) {
      counters1 = (int[]) props.get(COUNTERS1_KEY);
    }
    if (props.get(COUNTERS2_KEY) != null) {
      counters2 = (Integer[]) props.get(COUNTERS2_KEY);
    }
  }
}

Using this code, when tigase.util.DataTypes.valueToString(Object) is called on the Integer[] varient it will throw an exception.

This is also a code bug and not just a philosophical issue because if you look in the tigase.util.DataTypes class you explicitly add object and non-object arrays in the @typesMap@:

typesMap.put(Integer[].class.getName(), 'i');
typesMap>.put(int[].class.getName(), 'i');
</pre

however @tigase.util.DataTypes.valueToString(Object)@ is not able to deal with the object array variants; the version I wrote does.

(I know that arrays are objects, when I say "object array variant" it means "an array of objects" and "non-object array variant" means "an array of primitives").

Performance and resources usage reducing motivated many of our decision on how to design and implement certain parts of the code.

Therefore, reason why this method accepts arrays with only primitive types is performance and memory preservation. So this is not really a bug but intentional decision. So this is how it works.

I appreciate your explanation and I generally agree with the idea to make the code more flexible and allow it to handle more use-cases but in case of Tigase serve I prefer to push towards more efficient data types.

Artur Hefczyc wrote:

Performance and resources usage reducing motivated many of our decision on how to design and implement certain parts of the code.

Therefore, reason why this method accepts arrays with only primitive types is performance and memory preservation. So this is not really a bug but intentional decision. So this is how it works.

I appreciate your explanation and I generally agree with the idea to make the code more flexible and allow it to handle more use-cases but in case of Tigase serve I prefer to push towards more efficient data types.

Ok, I can understand if it is by design for performance issues, but then in this case remove the code in tigase.util.DataTypes where you add object arrays to typesMap

typesMap.put(Long[].class.getName(), 'l');
typesMap.put(Integer[].class.getName(), 'i');
typesMap.put(Boolean[].class.getName(), 'b');
typesMap.put(Float[].class.getName(), 'f');
typesMap.put(Double[].class.getName(), 'd');

since it doesn't make sense to have those there if they re not supported (it only adds confusion). Also you should mention in the docs that for arrays for types that have a primitive equivalent that you only support arrays of primitives. As a side note, not sure if this is by design too but your code, at least tigase.util.DataTypes.valueToString(Object) via the default case in the @switch@, supports @Object[]@. If I take what you said above, it shouldn't support other object arrays than @String[]@.

Ok, I looked at your code and our code and on the second thought, I think it actually makes sense to add your suggestions to the Tigase code and make the method more flexible.

Artur Hefczyc wrote:

Ok, I looked at your code and our code and on the second thought, I think it actually makes sense to add your suggestions to the Tigase code and make the method more flexible.

Ok, great, thx.

OK, one more question/concern - what about reading preferences from file (and counterpart of that - dumping settings) - how should we treat [i] - as array of int or @Integers@? Or should we introduce another type differentiation?

Wojciech, I think we should only change the code which "exports" variables to String as indicated in the original post. This method is used for dumping config to file as far as I remember. I do not want to alter logic which is reading preferences from a config file.

But imagine following configuration:

test/counters1[i]=1, 2, 3

We can assume this will be int array and as such should be handled in the component (as it's done right now).

Now following cases:

• someone uses Integer array, it's exported to file the same, then someone may want to use it in the configuration and by doing so it wont work because component would expect Integer array instead of resulting int array

• how to configure such component (requiring Integer array) from @init.properties@? Without any changes it won't work because it will always result in int array.

We use, everywhere in our code arrays of int for the configuration and this is how are API works and this is what our code expects. So, I do not think this is a problem that somebody want to use arrays of Integers instead. This is his worry how to load the property correctly. As long as our API is consistent and gives predictable results it is OK.

I've applied the patch.

On comment though.

Artur Hefczyc wrote:

We use, everywhere in our code arrays of int for the configuration and this is how are API works and this is what our code expects. So, I do not think this is a problem that somebody want to use arrays of Integers instead. This is his worry how to load the property correctly. As long as our API is consistent and gives predictable results it is OK.

OK, my previous comment implied that reading property from file is also somehow part of an "API" and if we allow to use object arrays in getDefaulst()@/@setProperties() then we should also support reading such from file.

No feedback, I am assuming it is working OK now. Closing.