search

Kevin-Jin / mmap

Forked from https://r-forge.r-project.org/scm/?group_id=648
1 stars 1 forks source link

Refactor Ctype constructors #12

Closed Kevin-Jin closed 7 years ago

Kevin-Jin commented 7 years ago

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.

Kevin-Jin commented 7 years ago

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.

Kevin-Jin commented 7 years ago

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.

Kevin-Jin commented 7 years ago

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

Kevin-Jin commented 7 years ago

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

Kevin-Jin commented 7 years ago

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.