Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsers for pointers #7

Open
alexandrebini opened this issue Jan 19, 2018 · 5 comments
Open

Parsers for pointers #7

alexandrebini opened this issue Jan 19, 2018 · 5 comments

Comments

@alexandrebini
Copy link

Hi @rdumont,

We're using assistdog here on CashCowPro. Thanks for it =)

We feel the need to add parsers for pointers, so we could handle nullable variables. Right now we're using N/A to identify the nil. So our tables look like this:

Given we have the following products:
    | Title                             | CreatedAt            | UpdatedAt            |
    | Paladone Festive Photo Booth Game | 2017-12-16T00:00:00Z | 2017-12-17T00:00:00Z |
    | N/A                               | 2017-12-18T00:00:00Z | N/A                  |

What do you think about this approach? I would be happy to raise an PR =]

If you want to take a look at our current implementation: https://github.com/cashcowpro/assistdog/pull/1/files

Thanks!

@alexandrebini
Copy link
Author

@rdumont ?

@rdumont
Copy link
Owner

rdumont commented Jan 25, 2018

Hi, @alexandrebini! I'm sorry for not noticing your PR earlier.

Interesting approach, using N/A. However, I feel like that is too arbitrary to be part of the default parsers and comparers. I see, however, that you added float32 to the list, which should be in the defaults.

The point of the Assist struct is exactly to allow each dev to customize it according to their needs. Do you think that you could write a PR that documents examples of how to implement the nullable variables, just the way you did it?

@alexandrebini
Copy link
Author

Hey @rdumont! Sure I will :)

About the N/A approach honestly I'm also not sure if is the best approach. One thing that I thought was to instead of using N/A, just assume that empty rows are nil for pointers. In this case we would lose the option to really test empty strings.

Other approach that I thought was to just use the word nil instead of N/A.

What do you think about it?

@alexandrebini
Copy link
Author

@rdumont ? 😃

@rdumont
Copy link
Owner

rdumont commented Feb 8, 2018

@alexandrebini, I'm sorry for being terrible at keeping up here! :(

I think that if the N/A approach works for you, you should keep using it. But i don't like the idea of, by default, giving a special meaning to any arbitrary string, whether it is nil or N/A, or empty. IMO, this is the kind of thing that, when a person feels the need for it, they will find a solution. But, if they don't need it, the hidden behavior might come in as an unpleasant surprise.

So I think that just documenting your solution is the way to go!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants