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

Posexional row as struct #29

Merged
merged 5 commits into from
Feb 12, 2021
Merged

Conversation

GPrimola
Copy link
Contributor

@GPrimola GPrimola commented Feb 4, 2021

This is a proposal for #11.

Obs 1: There's no breaking changes.
Obs 2: I apologize about the size of this PR. 😅

Major Changes

  • Option [:as_struct] to PosexionalRow.__using__/1 macro. Not breaking.
  • New field Posexional.Field.Field that parses field type text. It behaves like Ecto.Schema.field/3. Not breaking.

Examples

PosexionalRow.__using__/1 with option [:as_struct] and PosexionalRow.field/4 with simple data types:

Suppose we have the following row definition to be mapped to a struct:

defmodule Row1 do
  use PosexionalRow, [:as_struct]

  guesser &__MODULE__.guesser/1

  fixed_value "row1"
  field :id, :id, 3
  field :binary_id, :binary_id, 3
  field :integer, :integer, 2
  field :float, :float, 6
  field :boolean, :boolean, 5
  field :string, :string, 4
  field :binary, :binary, 4

  def guesser("row1" <> _), do: true
  def guesser(_), do: false
end

defmodule StructRowFile do
  use PosexionalFile

  row Row1
end

and a positional file (example.txt - not included) with the content row1123ABA00432.99falseJohn Doe. Then

StructRowFile.read("row1123ABA00432.99falseJohn Doe")
=>
[
  %Row1{
    binary: "Doe",
    binary_id: "ABA",
    boolean: false,
    float: 432.99,
    id: 123,
    integer: 0,
    string: "John"
  }
]

PosexionalRow.__using__/1 with option [:as_struct] and PosexionalRow.field/4 with complex data types:

defmodule Row2 do
  use PosexionalRow, [:as_struct]

  guesser &__MODULE__.guesser/1

  field :custom, :custom, 4, parser: &String.to_atom/1
  field :array, :array, 4, separator: ""
  field :list, :list, 7, separator: "|"
  field :decimal, :decimal, 8
  field :date, :date, 8, format_in: "{0M}{0D}{YYYY}"
  field :time, :time, 8, format_in: "%H%M%S%f"
  field :time2, :time, 6, format_in: "%H%M%S"
  field :datetime, :datetime, 14, format_in: "%d%m%Y%H%M%S", timezone: "America/Sao_Paulo"
  field :naive_dt, :naive_datetime, 23, format_in: "%d/%m/%Y@%H:%M:%S.%L"
  field :utc_dt, :utc_datetime, 23, format_in: "%Y-%m-%d %H:%M:%S.%L"

  def guesser("afoo" <> _), do: true
  def guesser(_), do: false
end

defmodule StructRowFile do
  use PosexionalFile

  row Row2
end

StructRowFile.read("afoo1234cat|dog123.45  01312021235959990745001209198807450031/12/2021@23:59:59.9992022-01-01 00:00:00.000")
=>
[
%Row2{
     array: ["1", "2", "3", "4"],
     custom: :afoo,
     date: ~D[2021-01-31],
     datetime: %DateTime{
       year: 1988,
       month: 9,
       day: 12,
       hour: 7,
       minute: 45,
       second: 00,
       utc_offset: -10800,
       time_zone: "America/Sao_Paulo",
       std_offset: 0,
       zone_abbr: "-03"
     },
     decimal: 123.45,
     list: ["cat", "dog"],
     naive_dt: ~N[2021-12-31 23:59:59.999],
     time: ~T[23:59:59.99],
     time2: ~T[07:45:00],
     utc_dt: ~U[2022-01-01 00:00:00Z]
   }
]

If you want more information about the behaviour, you can check the test/posexional/field/field_test.exs file where there are examples of bad input and how it's been handled.

Please, feel free to comment, propose changes, code refactor, variable, module, function and option naming, etc. I'll be glad to tackle everything!

Congrats about the lib, though! It's been very helpful! 😄

Copy link
Contributor

@neslinesli93 neslinesli93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work, for real ❤️

Unfortunately, our CI service is purposely disabled for public projects, so it can't be run on this PR 😢
I've tried locally to run credo and dialyzer and I got a bunch of errors. You can try mix check and see that credo complains about single pipes, and some other stuff.

Apart from that, I'm not a huge fan of having a Posexional.Field.Field module, mostly for the repetition of words and because imho it's not that self-explanatory. Maybe something like TypedField would be better? I like the reference to Ecto, but the repetition kills me :D

lib/posexional/field/field.ex Outdated Show resolved Hide resolved
lib/posexional/row.ex Outdated Show resolved Hide resolved
…stency on Posexional.Row.read/2. Fixed credo complains
@GPrimola
Copy link
Contributor Author

GPrimola commented Feb 9, 2021

Hey @neslinesli93,

Thank you for your review! I've already tackled everything you pointed out!
And agree, Posexional.Field.Field was a bad name! Thanks for the suggestion! ;)

If you have anything more to add, please feel free to do it! :)

chess4ever
chess4ever previously approved these changes Feb 11, 2021
@chess4ever
Copy link
Collaborator

Hi @GPrimola ! You have done a really great work here! Thank you for you contribution!
We would like to merge your PR a soon as possible since it is valuable.

I have run dialyzer locally (in the container) and there are still some issues (I count 9 of those).

Let us know if you need help to solve them! We are glad to help!

Thank you again!

@chess4ever
Copy link
Collaborator

@GPrimola is it ready to be merged?

@GPrimola
Copy link
Contributor Author

Hey @chess4ever, thanks!

Actually I missed letting dialyzer run fully.
Now I think everything should be fine!

Thank you!

@mbusi mbusi merged commit 6c3d5b8 into primait:master Feb 12, 2021
@mbusi mbusi linked an issue Feb 12, 2021 that may be closed by this pull request
GPrimola added a commit to GPrimola/posexional that referenced this pull request Feb 12, 2021
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

Successfully merging this pull request may close these issues.

Introduce option to get structs instead of keyword lists when reading
4 participants