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

Replace the magic getters/setters with non-magic ones #130

Open
oliverklee opened this issue Mar 15, 2019 · 6 comments
Open

Replace the magic getters/setters with non-magic ones #130

oliverklee opened this issue Mar 15, 2019 · 6 comments
Milestone

Comments

@oliverklee
Copy link
Collaborator

The AbstractType::__get and AbstractType::__set do not allow static analysis for cases where they are used. I'd like to deprecate them and replace them with explicit get and set methods together with explicitly-typed getAsString, setAsBoolean etc. methods.

@mjaschen What do you think?

@oliverklee
Copy link
Collaborator Author

(And also, Prophecy can only stub/mock existing methods, not magic method calls.)

@mjaschen
Copy link
Owner

Type properties would be accessed like $type->getAsString('client_id') or $type->get('client_id)`, right?

@oliverklee
Copy link
Collaborator Author

Yes, exactly: with getAsString already typed as string, and with get with mixed type/untyped.

@mjaschen
Copy link
Owner

Sounds like a good idea.

Maybe we can add specific getters/setters for each field on a per type basis in a later step (getClientId(), setFirstname()). That'd be a lot of boilerplate code but would also simplify working with the library.

@oliverklee
Copy link
Collaborator Author

Maybe we can add specific getters/setters for each field on a per type basis in a later step

Sounds good.

Then I propose we move this to post-1.0 (i.e., 2.0 as this will be breaking as well).

@oliverklee oliverklee added this to the 2.0.0 milestone Mar 18, 2019
@mjaschen
Copy link
Owner

mjaschen commented Mar 21, 2019

(for later reference)

The Collmex API uses sixseven different data types[1]:

  • alpha-numerical (i.e. “strings”)
  • numerical, integer: an ordinary integer value
    • also used for IDs: in this case it can be followed by a space and a string which is the readable name for the referenced record. Example: 5 and 5 PayPal are two representations for the same value. The appended space + string can be omitted when using in a setter. We should make sure to process it when parsing a CSV document though (maybe even expose it in a special getter method).
  • numerical, decimal: a decimal value with up to 3 decimals. Collmex returns (and expects) values in the german localization (comma as decimal separator); we should take care of conversions between PHP's float values and Collmex' numerical values in the setters/getters
  • money: a decimal value with 2 decimals, same notes as on “numerical, decimal” apply here; maybe we can use moneyphp/money to store money values internally
  • date: a date string in the format YYYYMMDD or DD.MM.YYYY; we should use only the former format when creating API data; dates should stored as DateTime internally
  • time: a time string in the format HHMMSS
  • special: if a field contains the value (NULL) (literalt string) it won’t be changed when a record is sent to Collmex

[1] https://www.collmex.de/cgi-bin/cgi.exe?1005,1,help,daten_importieren_datentypen_felder

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

No branches or pull requests

2 participants