Code Review - Applying functions on custom types

[Sofiane Cherchalli]

Hi!

I'm a noob in Go and I need some guidance/help on this: https://play.golang.org/p/0TGzKiYQZn

Basically I'm implementing a CSV parser, and applying transformations on column value.

In last part of the code I'm trying to apply a function on CSVFloat type which satisfies Valuer interface, but I got a compiler error.

In Scala language, this could be done by using map function, but how to do it in Golang?

Thanks.

[silviu...@gmail.com]

Before: myfn := func(v CSVFloat) CSVFloat { return v }

After: myfn := func(v Valuer) Valuer { return v }

[Sofiane Cherchalli]

Hi Silviu,

Thanks for the reply.

Basically I want to kinda functional map on my custom types by applying functions on base value or struct values.

What if I want to for instance:

- Multiply the float64 value inside CSVFloat by 2 ?

- or Replace a custom type value with another one from the same type?

Thanks

[roger peppe]

I'm not convinced that holding all your values in a uniform way

is going to be that helpful for you. You might be better using reflection

to map the columns into a struct type (there's probably a package

out there already that can do that).

However, to answer without questioning the whole premise:

You can't pass a function on a specific type to the more generally

typed func(Valuer)Valuer because the value in the column might not

be of the specific type - what should happen if the column is a string

and you pass func(CSVFloat)CSVFloat to Apply?

Here's your code made to work, with some arguably redundant stuff

removed. The Transformer type seemed unnecessary, as Apply

and RemoveColumn both work with the row in place. The Type field

in the Column struct seemed unnecessary, as the type is implied by

the value in the column. Also, the whole notion of Type seemed

a bit redundant as CSV files have no notion of type, and it seems like

you want to support custom types. The CSV prefix on the type names

seemed unnecessary, as this would probably be in a package with

some kind of csv-related name.

https://play.golang.org/p/9mSfG1m4VZ

  cheers,

    rog.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[silviu...@gmail.com]

Hi Sofiane,

Answering the "what type" question is pretty much unavoidable. 

You can embed that forking logic inside the "on the fly" function, like in Roger's example (using a switch v := v.(type) construct) or you can use reflect.

Alternatively, you can group your transformation functions into functionality buckets, using maps: map[string]TFunc (where TFunc is func(Valuer) Valuer like you wanted).

Each of your types (CSVString or CSVFloat, etc) would be "tied" to such a map, returned by a new method in the Valuer interface: TFuncs() map[string]TFunc

Quick example here:  https://play.golang.org/p/ZnqJVw0Wzq

I just added some extra stuff to your code. I didn't bother trying to understand if you wanted your rows mutated for each operation, or copied, or such. That's your decision.

For chaining transformers, please note that I changed your r := Row { ... }  to var r Transformer = &Row{ ... }

Keep in mind that a lot of functional paradigms do not apply cleanly to Go. For readability purposes, a simple for loop works wonders and it's oftentimes more readable :)

cheers,

silviu

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

[Sofiane Cherchalli]

Hi Rog,

Please inline comments.

Thx

On Thursday, July 20, 2017 at 1:58:09 PM UTC+2, rog wrote:

I'm not convinced that holding all your values in a uniform way

is going to be that helpful for you. You might be better using reflection

to map the columns into a struct type (there's probably a package

out there already that can do that).

However, to answer without questioning the whole premise:

You can't pass a function on a specific type to the more generally

typed func(Valuer)Valuer because the value in the column might not

be of the specific type - what should happen if the column is a string

and you pass func(CSVFloat)CSVFloat to Apply?

Couldn't do checking with for instance reflection?

 

Here's your code made to work, with some arguably redundant stuff

removed. The Transformer type seemed unnecessary, as Apply

and RemoveColumn both work with the row in place. The Type field

in the Column struct seemed unnecessary, as the type is implied by

the value in the column. Also, the whole notion of Type seemed

a bit redundant as CSV files have no notion of type, and it seems like

you want to support custom types. The CSV prefix on the type names

seemed unnecessary, as this would probably be in a package with

some kind of csv-related name.

Is it possible to solve the problem without using type assertion?

 

https://play.golang.org/p/9mSfG1m4VZ

  cheers,

    rog.

On 20 July 2017 at 08:05, Sofiane Cherchalli <sofi...@gmail.com> wrote:

Hi Silviu,

Thanks for the reply.

Basically I want to kinda functional map on my custom types by applying functions on base value or struct values.

What if I want to for instance:

- Multiply the float64 value inside CSVFloat by 2 ?

- or Replace a custom type value with another one from the same type?

Thanks

On Thursday, July 20, 2017 at 5:09:40 AM UTC+2, Silviu Capota Mera wrote:

Before: myfn := func(v CSVFloat) CSVFloat { return v }

After: myfn := func(v Valuer) Valuer { return v }

On Wednesday, 19 July 2017 16:48:07 UTC-4, Sofiane Cherchalli wrote:

Hi!

I'm a noob in Go and I need some guidance/help on this: https://play.golang.org/p/0TGzKiYQZn

Basically I'm implementing a CSV parser, and applying transformations on column value.

In last part of the code I'm trying to apply a function on CSVFloat type which satisfies Valuer interface, but I got a compiler error.

In Scala language, this could be done by using map function, but how to do it in Golang?

Thanks.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

[Sofiane Cherchalli]

Hi Rog,

Please inline comments.

Thx

On Thursday, July 20, 2017 at 1:58:09 PM UTC+2, rog wrote:

I'm not convinced that holding all your values in a uniform way

is going to be that helpful for you. You might be better using reflection

to map the columns into a struct type (there's probably a package

out there already that can do that).

However, to answer without questioning the whole premise:

You can't pass a function on a specific type to the more generally

typed func(Valuer)Valuer because the value in the column might not

be of the specific type - what should happen if the column is a string

and you pass func(CSVFloat)CSVFloat to Apply?

Couldn't do checking with for instance reflection?

 

Here's your code made to work, with some arguably redundant stuff

removed. The Transformer type seemed unnecessary, as Apply

and RemoveColumn both work with the row in place. The Type field

in the Column struct seemed unnecessary, as the type is implied by

the value in the column. Also, the whole notion of Type seemed

a bit redundant as CSV files have no notion of type, and it seems like

you want to support custom types. The CSV prefix on the type names

seemed unnecessary, as this would probably be in a package with

some kind of csv-related name.

Is it possible to solve the problem without using type assertion?

 

https://play.golang.org/p/9mSfG1m4VZ

  cheers,

    rog.

On 20 July 2017 at 08:05, Sofiane Cherchalli <sofi...@gmail.com> wrote:

Hi Silviu,

Thanks for the reply.

Basically I want to kinda functional map on my custom types by applying functions on base value or struct values.

What if I want to for instance:

- Multiply the float64 value inside CSVFloat by 2 ?

- or Replace a custom type value with another one from the same type?

Thanks

On Thursday, July 20, 2017 at 5:09:40 AM UTC+2, Silviu Capota Mera wrote:

Before: myfn := func(v CSVFloat) CSVFloat { return v }

After: myfn := func(v Valuer) Valuer { return v }

On Wednesday, 19 July 2017 16:48:07 UTC-4, Sofiane Cherchalli wrote:

Hi!

I'm a noob in Go and I need some guidance/help on this: https://play.golang.org/p/0TGzKiYQZn

Basically I'm implementing a CSV parser, and applying transformations on column value.

In last part of the code I'm trying to apply a function on CSVFloat type which satisfies Valuer interface, but I got a compiler error.

In Scala language, this could be done by using map function, but how to do it in Golang?

Thanks.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

[Sofiane Cherchalli]

Hi Silviu,

Thanks for the example.

I have some questions popping up in my mind: Is my design wrong? Is your example and/or Roger's the most idiomatic way to do it in Go?

Is it wrong to use type assertions in Go?

Thanks.

[silviu...@gmail.com]

Hi Sofiane,

"Is my design wrong?"

Without a bigger picture of what your final aim is, it's hard for an external observer to tell you if your design is right or wrong.

I was unable to fully grasp the need for that intermediate (CSV prefixed) structs in-between raw csv and real domain types, so I just assumed you're working on some sort of a streaming processor / data importer of sorts where values were represented by basic types (strings, numbers, etc).

"Is your example and/or Roger's the most idiomatic way to do it in Go?"

I think Roger's and my example were targeted towards helping you figure out a specific problem you had: supplying custom functionality to that transformer of yours, that you can apply as needed on the original values.

He used a type switch and I used a type assertion. A third way (which we did not cover, but he mentioned) is using reflection (the reflect package).

No matter what, using your design with the Valuer interface, you need to explicitly code the type check and get the concrete type, in order to do different things to a string versus a float.

"Is it wrong to use type assertions in Go?"

Type assertions are a fundamental part of Go. There's nothing wrong using them: https://tour.golang.org/methods/15

Regarding "idiomatic": Maybe it's just that you are trying to drive the Go vehicle using a Scala driver's licence, and you need more time to get used to what can and what cannot be done ? 

I would urge you to keep at it, try several variations, and in 2-3 months tops you'll be able to naturally calibrate your perspective, and tell which code feels idiomatic and which doesn't.

Cheers,

silviu

[Egon]

In a uniform column situation I would model a single Column, effectively have:

type StringColumn struct{ Cells []string }

type FloatColumn struct{ Cells []int }

This should also make many of the operations easier to implement.

But since I'm unclear what the end goal of this solution is, I'm not sure whether this is sufficient or whether there are better designs.

+ Egon

[Sofiane Cherchalli]

Hi Silviu,

On Sunday, July 23, 2017 at 2:11:29 AM UTC+2, Silviu Capota Mera wrote:

Hi Sofiane,

"Is my design wrong?"

Without a bigger picture of what your final aim is, it's hard for an external observer to tell you if your design is right or wrong.

I was unable to fully grasp the need for that intermediate (CSV prefixed) structs in-between raw csv and real domain types, so I just assumed you're working on some sort of a streaming processor / data importer of sorts where values were represented by basic types (strings, numbers, etc).

Yes , the idea is to do streaming of incoming rows. Working with CSVxxx, which are mainly custom wrapper of basic types, allows me to take advantage of go interfaces to add behavior. I'm not sure now if it's the right approach.

 

"Is your example and/or Roger's the most idiomatic way to do it in Go?"

I think Roger's and my example were targeted towards helping you figure out a specific problem you had: supplying custom functionality to that transformer of yours, that you can apply as needed on the original values.

He used a type switch and I used a type assertion. A third way (which we did not cover, but he mentioned) is using reflection (the reflect package).

No matter what, using your design with the Valuer interface, you need to explicitly code the type check and get the concrete type, in order to do different things to a string versus a float.

"Is it wrong to use type assertions in Go?"

Type assertions are a fundamental part of Go. There's nothing wrong using them: https://tour.golang.org/methods/15

Clearly I have a lot of things to learn, I'll have a look at type assertions and switched. 

Regarding "idiomatic": Maybe it's just that you are trying to drive the Go vehicle using a Scala driver's licence, and you need more time to get used to what can and what cannot be done ? 

I would urge you to keep at it, try several variations, and in 2-3 months tops you'll be able to naturally calibrate your perspective, and tell which code feels idiomatic and which doesn't.

I guess yes I'm biased :)

[roger peppe]

On 24 July 2017 at 05:10, Sofiane Cherchalli <sofi...@gmail.com> wrote:

Hi Silviu,

On Sunday, July 23, 2017 at 2:11:29 AM UTC+2, Silviu Capota Mera wrote:

Hi Sofiane,

"Is my design wrong?"

Without a bigger picture of what your final aim is, it's hard for an external observer to tell you if your design is right or wrong.

I was unable to fully grasp the need for that intermediate (CSV prefixed) structs in-between raw csv and real domain types, so I just assumed you're working on some sort of a streaming processor / data importer of sorts where values were represented by basic types (strings, numbers, etc).

Yes , the idea is to do streaming of incoming rows.

That sounds like a secondary goal to me. ISTM that you might be trying to

write a general package before you've got a decent use case.

What's an example of an actual problem that you're trying to solve

here?

I suspect that you might find that using encoding/csv directly can be pretty

good for streaming and processing CSV.

  cheers,

    rog.

[Sofiane Cherchalli]

Yes, I'm trying to stream CSV values encoded in strings. A schema defines a type of each value, so I have to parse values to verify they match the type. Once validation is done, I apply functions on each value.

Working with basic types instead of custom types that wrap the basic types, sounds to me that it would make it difficult to have a good abstraction... What do you think?

[Sofiane Cherchalli]

Hi Whom,
Yes you could with columnar CSV and apply functions to column values, something basically similar to what does spark. In my case I receive streams of rows.
Thx

[roger peppe]

On 24 July 2017 at 23:21, Sofiane Cherchalli <sofi...@gmail.com> wrote:

Yes, I'm trying to stream CSV values encoded in strings. A schema defines a type of each value, so I have to parse values to verify they match the type. Once validation is done, I apply functions on each value.

Is the schema dynamically or statically specified? That is, do you know

in advance what the schema is, or do are you required to write

general code that deals with many possible schemas?

[Sofiane Cherchalli]

The schema is statically specified. The values always arrive in a defined order. Each value has a defined type.

[Diego Medina]

I think we have a similar setup to what you are trying to do, we also started with Scala and about 3 years ago we moved it to Go (still use Scala for other parts of our app).

While working in Scala and other languages you are encourage to abstract things as much as you can, in Go it is often better to just address the issues/requirements at hand and be clear on what you are doing.
In our case we define a struct that has the expected fields and types for each column, and as we walk each row, we check that we get the expected type, then it's a matter of cleaning/adjusting values as we need to, assign the result of this cell to a variable and continue with the rest of the cells on this row, once done, we initialize our struct and save it to the database, move to the next row and repeat.

Hope it helps.

[Sofiane Cherchalli]

Hi Medina,

Sorry I was on vacations.

So do you mean the way to do it is to hardcode most of functionality. No need to use custom types, interfaces. Just plain text parsing?

In that case, how easy is it to evolve or refactor the code?

Thanks

[Diego Medina]

Hope you had a goo time on vacation.

No need for custom types, specially with csv files, all you get are "string" type, but you know that some columns are actually float values, or ints or plain text. So we have several functions that "clean, format, etc" different kinds of data, and just call them with the value we get. For example, we have:

func parseDateTimeOrZero(in string) time.Time {

    rtd := strings.TrimSpace(in)
    loc, _ := time.LoadLocation("America/New_York")

    var t = time.Date(1, 1, 1, 0, 0, 0, 0, time.UTC)
    for _, format := range dateLayouts {
        stamp, er := time.ParseInLocation(format, rtd, loc)
        if er == nil {
            t = stamp
            break
        }
    }
    return t
}

dateLayouts is a slice of all the date time formats we support, we loop over them until we get one that works, if not, we return the default value (for our app this makes sense, in other cases we could return an error)

and while walking the csv, we just do:

if headers.versusDate != optionalErrorValue {
  versusDate = parseDateTimeOrZero(record[headers.versusDate])
}

// optionalErrorValue defaults to a high enough number that if our column matcher hasn't updated the key index, it means we skip it, (we then have a diff rule for mandatory fields)

so, instead of creating a new interface DateTimeColumn with a parse function, it's just a stand alone function that is easy to understand, easy to test.

We have been evolving this part of the tool for years, we have had different developers work on it, adding new parsing rules, editing existing, making some of them more flexible, and it hasn't been an issue.

The thought process is, I have a value and I need to clean it, you check the list of functions to see if there is anything that looks like it does what you want, if not, add a new one vs, let's find an interface or type that

may have the things we need, then let's initialize that struct and call the method on it.

BTW, when I first starting using Go, after working on scala for about 5 years, it felt really wrong to have so many top level functions, but after a while you see how there is nothing wrong with top level functions.

Hope this helps.

Thanks

Diego

--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/b0UQrfolaiY/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Diego Medina
Go Consultant
di...@fmpwizard.com
https://blog.fmpwizard.com/

[Egon]

I've suggested this approach based in the slack, adding it here as well to make it searchable.

Implement Reader/Writer such that you can do:

func ProcessLine(in *Reader, out *Writer) error {

    start, finish, participant := in.Float64(), in.Float64(), in.String()

    if err := in.Err(); err != nil {

        return err

    }

    out.String(participant)

    out.Float64(finish - start)

    return nil

}

+ Egon

[Sofiane Cherchalli]

Thanks Medina!

I'll give it a try

To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.