Reminder: Two days left to review Jesse Phillips' CSV Parser

Timon Gehr timon.gehr at gmx.ch
Fri Nov 11 10:34:16 PST 2011


On 11/11/2011 06:49 AM, Jesse Phillips wrote:
> On Thu, 10 Nov 2011 15:04:47 +0100, Jonas Drewsen wrote:
>
>> On 10/11/11 02.02, dsimcha wrote:
>>> As a reminder, the review of Jesse Phillips' CSV parser ends at the end
>>> of Friday and will be followed by one week of voting. Please speak up
>>> now about any remaining issues.
>>
>> I've not have a look at the code itself yet. But a couple of comments
>> about the API:
>>
>> auto csvReader(Contents = string, Range, Heading, Separator =
>> char)(Range input, Heading heading, Separator delimiter = ',', Separator
>> quote = '"');
>>
>> 1, I was under the impression that declaring the return value as auto
>> for std library is not good because it does not document what the return
>> value actually is. Or maybe it is good enough that it is mentioned in
>> the comments under "Returns: ..."?
>
> Quite simply, I don't know what it returns. The discussion was on
> examples using auto, which I also do for basically the same reason.
> Everyone was in agreement that complicated template returns deserve the
> use of auto, but I didn't get an answer why _complicated_ types get auto
> but simple ones can't use it. Luckily mine is complicated.
>
>> 2, The requirement for passing "cast(string[]) null" as the heading
>> parameter to signal that heading is there but should be ignored seems a
>> bit awkward.
>> Maybe an overload for csvReader where you remove the constraints:
>> isForwardRange!Heading&&  isSomeString!(ElementType!Heading) and set
>> is(Heading:void*) instead. And in there just call the normal csvReader
>> with cast(string[])null.
>
> You should be able to pass null, but there is a bug in dmd. You might
> have a good workaround for that though.
>
> Well, it seems I just have to assume that void* means they are passing
> null. Thank you.
>

Kenji has already written a pull request to give null it's own type. 
Therefore, make sure not to hardcode void*, use typeof(null).


More information about the Digitalmars-d mailing list