Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Why not use ArrayString class in factory method of Array class? #1077

Open
Yaqiang opened this issue Mar 31, 2018 · 8 comments
Open

Why not use ArrayString class in factory method of Array class? #1077

Yaqiang opened this issue Mar 31, 2018 · 8 comments

Comments

@Yaqiang
Copy link

Yaqiang commented Mar 31, 2018

factory method in Array class:

static public Array factory(DataType dtype, Index index, Object storage) {
    switch (dtype) {
      case DOUBLE:
        return ArrayDouble.factory(index, (double[]) storage);
      case FLOAT:
        return ArrayFloat.factory(index, (float[]) storage);
      case CHAR:
        return ArrayChar.factory(index, (char[]) storage);
      case BOOLEAN:
        return ArrayBoolean.factory(index, (boolean[]) storage);
      case ENUM4:
      case UINT:
      case INT:
        return ArrayInt.factory(index, dtype.isUnsigned(), (int[]) storage);
      case ENUM2:
      case USHORT:
      case SHORT:
        return ArrayShort.factory(index, dtype.isUnsigned(), (short[]) storage);
      case ENUM1:
      case UBYTE:
      case BYTE:
        return ArrayByte.factory(index, dtype.isUnsigned(), (byte[]) storage);
      case ULONG:
      case LONG:
        return ArrayLong.factory(index, dtype.isUnsigned(), (long[]) storage);
      case STRING:
        return ArrayObject.factory(dtype, String.class, false, index, (Object[]) storage);
      case STRUCTURE:
        return ArrayObject.factory(dtype, StructureData.class, false, index, (Object[]) storage);
      case SEQUENCE:
        return ArrayObject.factory(dtype, StructureDataIterator.class, false, index, (Object[]) storage);
      case OPAQUE:
        return ArrayObject.factory(dtype, ByteBuffer.class, false, index, (Object[]) storage);
    }

ArrayObject was used if DataType is STRING. So why not use ArrayString class?

@cwardgar
Copy link
Contributor

cwardgar commented Apr 2, 2018

Not only are we not using ArrayString in that factory method, we're not using it anywhere. And maybe that's fine, because it doesn't actually provide any functionality that ArrayObject does not.

So maybe it's best to just nuke it. Anyone disagree?

@cwardgar
Copy link
Contributor

cwardgar commented Apr 2, 2018

@DennisHeimbigner It looks like you were the one who added the class originally. Any thoughts?

@lesserwhirls
Copy link
Collaborator

I think the reason we have the factory return an ArrayObject is because you'd have to make some assumptions about how the string is stored. For example, are the strings stored in columns or rows Currently, the netCDF Users Guide does not provide guidance on this specific issue, and it is not in any standard that I know of.

@Yaqiang
Copy link
Author

Yaqiang commented Apr 3, 2018

As a user at least I want to get DataType.STRING from the created string array by factory method.

@cwardgar
Copy link
Contributor

cwardgar commented Apr 3, 2018

@Yaqiang Currently, Array.factory(DataType.STRING, new int[1]).getDataType() == DataType.STRING.

@cwardgar
Copy link
Contributor

@DennisHeimbigner poke

@DennisHeimbigner
Copy link
Contributor

DennisHeimbigner commented Apr 11, 2018

you'd have to make some assumptions about how the string is stored. For example,
are the strings stored in columns or rows

I do not understand this point. what do columns and rows have to do with variable length strings?
I think we had a discussion a long time ago (but after John left) about ArrayString. I was in favor of it for consistency. You two wanted to stay with ArrayObject. Without looking, do any other types (user defined types) get represented as ArrayObject: enums, for example?

@lesserwhirls
Copy link
Collaborator

@DennisHeimbigner - yeah, I was way confused in my response. I was thinking of what to do if one wanted to try to promote a char array to string array - my bad!

So, I think sticking with ArrayObject in 4.6.x is a must for backward compatibility. However, as @Yaqiang pointed out, getting back an ArrayString would make most sense from a users point of view, and although we are not using it, I'd guess someone using netCDf-java is. Let's chat about this tomorrow at our meeting.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants