Kevin-Jin/mmap

Refactor Ctype constructors

Kevin-Jin opened this issue · 5 comments

I don't think that the length of Ctypes is used except for counting the number of columns in a struct. The char Ctype needs a length parameter but its semantics are different than the length parameters of other Ctypes (the passed length is stored into the bytes attribute so the length of the created vector is redundant). Grep through the code to verify this is the case and consider the removal of the parameter. It seems like some space is wasted whenever length is not 0 because a vector of the corresponding R type is allocated with the passed length.

It's also very confusing how Ctypes and vector types are mixed for the default value of mode in the as.mmap() overrides. It may be better just to use Ctypes all around e.g. mode=uchar() for as.mmap.raw() and mode=int32() for as.mmap.integer.

Alternatively, we can simply use mode=as.Ctype(x) instead.

Consider separating the two cases of char into two different classes entirely (e.g. char and string) because they are used for very different reasons.

Even before my modifications, this would have made sense because nul is not an attribute in the raw case but it is in the string case, so the char class isn't entirely consistent.

Consider removing the int24 and uint24 types because it is highly unlikely they will actually be used.

Pass ... to as.Ctype() in the as.mmap() overrides.

Keep the classes for the raw mode Ctypes as char and uchar to distinguish them from the integer mode Ctypes int8 and uint8. Rename the logi8 and logi32 classes to bool8 and bool32 to match the theme of using C types as class names.