Jump to page: 1 26  
Page
Thread overview
Review of Jesse Phillips's CSV Parser
Oct 28, 2011
dsimcha
Oct 28, 2011
Vladimir Panteleev
Oct 28, 2011
Jesse Phillips
Oct 28, 2011
Vladimir Panteleev
Oct 28, 2011
Piotr Szturmaj
Oct 28, 2011
Jacob Carlborg
Oct 28, 2011
Piotr Szturmaj
Oct 28, 2011
deadalnix
Oct 28, 2011
Jesse Phillips
Oct 28, 2011
dsimcha
Oct 28, 2011
Jesse Phillips
Oct 28, 2011
Jesse Phillips
Oct 28, 2011
Jacob Carlborg
Oct 28, 2011
Jesse Phillips
Oct 28, 2011
Jacob Carlborg
Oct 28, 2011
Ary Manzana
Oct 28, 2011
Jesse Phillips
Oct 28, 2011
Ary Manzana
Oct 28, 2011
Jesse Phillips
Oct 28, 2011
Walter Bright
Oct 29, 2011
bls
Oct 29, 2011
Jesse Phillips
Oct 30, 2011
bls
Oct 30, 2011
Jesse Phillips
Oct 30, 2011
Walter Bright
Oct 29, 2011
Jesse Phillips
Oct 30, 2011
Jonathan M Davis
Oct 30, 2011
Jesse Phillips
Nov 05, 2011
Somedude
Oct 30, 2011
Walter Bright
Oct 30, 2011
Jesse Phillips
Oct 30, 2011
Steve Teale
Oct 30, 2011
Walter Bright
Oct 31, 2011
Vladimir Panteleev
Nov 01, 2011
Jesse Phillips
Nov 01, 2011
Jonathan M Davis
Nov 02, 2011
Jacob Carlborg
Nov 11, 2011
Don
Nov 11, 2011
Jacob Carlborg
Nov 11, 2011
Vladimir Panteleev
Nov 02, 2011
Vladimir Panteleev
Nov 03, 2011
Jesse Phillips
Nov 05, 2011
Jesse Phillips
Nov 07, 2011
Jesse Phillips
Nov 08, 2011
Robert Jacques
Nov 11, 2011
Jesse Phillips
Nov 11, 2011
Jonathan M Davis
Nov 11, 2011
Jesse Phillips
Nov 11, 2011
Jonathan M Davis
Nov 11, 2011
Manu
Nov 12, 2011
Robert Jacques
Nov 16, 2011
Jesse Phillips
Nov 16, 2011
Robert Jacques
Nov 16, 2011
Jesse Phillips
Nov 07, 2011
dsimcha
Nov 10, 2011
Jonathan M Davis
Nov 11, 2011
Jesse Phillips
October 28, 2011
Formal review of Jesse Phillips's CSV parser module begins today and runs through November .  Following that, a vote will take place for one week.  Please post any comments about this library in this thread.

Docs:
http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html

Code:
https://github.com/he-the-great/phobos/tree/csv
October 28, 2011
On Fri, 28 Oct 2011 16:18:27 +0300, dsimcha <dsimcha@yahoo.com> wrote:

> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html

Documentation should have examples of basic usage at the top. It should be possible to figure out how to use a CSV parser within 10 seconds of looking at the docs.

It looks like this is only a CSV parser, but not a CSV formatter? That's pretty weird. It'd be like std.xml or std.json only having reading functionality.

-- 
Best regards,
 Vladimir                            mailto:vladimir@thecybershadow.net
October 28, 2011
dsimcha wrote:
> Formal review of Jesse Phillips's CSV parser module begins today and
> runs through November . Following that, a vote will take place for one
> week. Please post any comments about this library in this thread.
>
> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>
> Code:
> https://github.com/he-the-great/phobos/tree/csv

Could csvText!Struct(...) be generalized to also support Tuples and (eventually) classes?
October 28, 2011
On 10/28/2011 9:18 AM, dsimcha wrote:
> Formal review of Jesse Phillips's CSV parser module begins today and
> runs through November . Following that, a vote will take place for one
> week. Please post any comments about this library in this thread.
>
> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>
> Code:
> https://github.com/he-the-great/phobos/tree/csv

I'll kick this off with my review of the documentation (I'll review the implementation later):

"Header may be provided as first line in file" -> "A header may be provided as first line in file"?

csvText:  Shouldn't heading be a generic range of strings instead of a string[]?

Why should the order of the heading provided to csvText matter?  csvText should simply rearrange its results.

In csvNextToken:  "The expected use of this would be to create a parser. And may also be useful when handling errors within a CSV file."  Please re-word this sentence so it's grammatically correct.
October 28, 2011
On 2011-10-28 15:25, Piotr Szturmaj wrote:
> dsimcha wrote:
>> Formal review of Jesse Phillips's CSV parser module begins today and
>> runs through November . Following that, a vote will take place for one
>> week. Please post any comments about this library in this thread.
>>
>> Docs:
>> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>>
>> Code:
>> https://github.com/he-the-great/phobos/tree/csv
>
> Could csvText!Struct(...) be generalized to also support Tuples and
> (eventually) classes?

Sounds more like a serialization library with a CSV archive type to me.

-- 
/Jacob Carlborg
October 28, 2011
On 2011-10-28 15:18, dsimcha wrote:
> Formal review of Jesse Phillips's CSV parser module begins today and
> runs through November . Following that, a vote will take place for one
> week. Please post any comments about this library in this thread.
>
> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>
> Code:
> https://github.com/he-the-great/phobos/tree/csv

If a header is specified, is it possible to iterate over the content as an associative array, something like this:

string str = "a,b,c\nHello,65,63.63\nWorld,123,3673.562";
auto records = csvText(str, ["b"]);

foreach (header, value ; records) {}

Or accessing a value by header:

foreach (record ; records)
{
    auto value = record["a"];
    auto val = record.b; // perhaps using opDispatch as well
}

-- 
/Jacob Carlborg
October 28, 2011
Jacob Carlborg wrote:
> On 2011-10-28 15:25, Piotr Szturmaj wrote:
>> dsimcha wrote:
>>> Formal review of Jesse Phillips's CSV parser module begins today and
>>> runs through November . Following that, a vote will take place for one
>>> week. Please post any comments about this library in this thread.
>>>
>>> Docs:
>>> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>>>
>>> Code:
>>> https://github.com/he-the-great/phobos/tree/csv
>>
>> Could csvText!Struct(...) be generalized to also support Tuples and
>> (eventually) classes?
>
> Sounds more like a serialization library with a CSV archive type to me.
>

I suppose you misunderstood my intentions. Here's an example from the docs:

string str = "Hello,65,63.63\nWorld,123,3673.562";
struct Layout {
    string name;
    int value;
    double other;
}

auto records = csvText!Layout(str);

foreach(record; records) {
    writeln(record.name);
    writeln(record.value);
    writeln(record.other);
}

I want to occasionally write:

alias Tuple!(string, int, double) Layout; // instead of struct

It should be easy to extend, but it would be nice if we have had general construct in Phobos. So instead of is(Contents == struct) @ https://github.com/he-the-great/phobos/blob/csv/std/csv.d#L433 there should be isComposite!Contents and FieldTypeTuple should be generalized (if it isn't) to structs, classes and tuples. I don't know if Tuple as a struct doesn't return valid fields from FieldTypeTuple now.
October 28, 2011
Le 28/10/2011 16:09, Piotr Szturmaj a écrit :
> Jacob Carlborg wrote:
>> On 2011-10-28 15:25, Piotr Szturmaj wrote:
>>> dsimcha wrote:
>>>> Formal review of Jesse Phillips's CSV parser module begins today and
>>>> runs through November . Following that, a vote will take place for one
>>>> week. Please post any comments about this library in this thread.
>>>>
>>>> Docs:
>>>> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>>>>
>>>> Code:
>>>> https://github.com/he-the-great/phobos/tree/csv
>>>
>>> Could csvText!Struct(...) be generalized to also support Tuples and
>>> (eventually) classes?
>>
>> Sounds more like a serialization library with a CSV archive type to me.
>>
>
> I suppose you misunderstood my intentions. Here's an example from the docs:
>
> string str = "Hello,65,63.63\nWorld,123,3673.562";
> struct Layout {
> string name;
> int value;
> double other;
> }
>
> auto records = csvText!Layout(str);
>
> foreach(record; records) {
> writeln(record.name);
> writeln(record.value);
> writeln(record.other);
> }
>
> I want to occasionally write:
>
> alias Tuple!(string, int, double) Layout; // instead of struct
>
> It should be easy to extend, but it would be nice if we have had general
> construct in Phobos. So instead of is(Contents == struct) @
> https://github.com/he-the-great/phobos/blob/csv/std/csv.d#L433 there
> should be isComposite!Contents and FieldTypeTuple should be generalized
> (if it isn't) to structs, classes and tuples. I don't know if Tuple as a
> struct doesn't return valid fields from FieldTypeTuple now.

That would be great !
October 28, 2011
On 10/28/11 10:18 AM, dsimcha wrote:
> Formal review of Jesse Phillips's CSV parser module begins today and
> runs through November . Following that, a vote will take place for one
> week. Please post any comments about this library in this thread.
>
> Docs:
> http://nascent.freeshell.org/programming/D/doc/phobos/std_csv.html
>
> Code:
> https://github.com/he-the-great/phobos/tree/csv

I like the idea of reading a CSV into a struct, or treating all fields as ints.

But, in my experience, CSVs are never perfect... because humans fill them, and humans aren't perfect or don't know "Oh, of course I must fill only numbers in this column, otherwise programs will explode".

So although it's a nice functionality, I think just returning everything as strings is more than enough.
October 28, 2011
dsimcha Wrote:

> I'll kick this off with my review of the documentation (I'll review the implementation later):
> 
> "Header may be provided as first line in file" -> "A header may be provided as first line in file"?
> 
> csvText:  Shouldn't heading be a generic range of strings instead of a string[]?

I should revisit that. It will still need to be random access and possible some others, but doesn't need to be an array.

> Why should the order of the heading provided to csvText matter?  csvText should simply rearrange its results.
> 
> In csvNextToken:  "The expected use of this would be to create a parser. And may also be useful when handling errors within a CSV file."  Please re-word this sentence so it's grammatically correct.

« First   ‹ Prev
1 2 3 4 5 6