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

Fix usage of hasXXX(), it now return bool only #1252

Merged
merged 4 commits into from Jun 12, 2020
Merged

Fix usage of hasXXX(), it now return bool only #1252

merged 4 commits into from Jun 12, 2020

Conversation

mvorisek
Copy link
Member

@mvorisek mvorisek commented Jun 8, 2020

in future, hasField might be introduced back but to return strictly bool value as expected, but it is no BC break now and will not be later - tryGetField will remain

@mvorisek mvorisek changed the title Refactor use of hasXXX() to tryGetXXX() Fix usage hasXXX(), it now return bool only Jun 11, 2020
@mvorisek mvorisek closed this Jun 11, 2020
@mvorisek mvorisek reopened this Jun 11, 2020
@mvorisek mvorisek changed the title Fix usage hasXXX(), it now return bool only Fix usage of hasXXX(), it now return bool only Jun 11, 2020
@mvorisek mvorisek closed this Jun 11, 2020
@mvorisek mvorisek reopened this Jun 11, 2020
Copy link
Collaborator

@georgehristov georgehristov left a comment

Choose a reason for hiding this comment

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

$row->get($field) should also work even if $field is a \atk4\data\Field. Any reason to change this?

@mvorisek
Copy link
Member Author

mvorisek commented Jun 12, 2020

@georgehristov $row->get($field) is was unified to accept only string field name at least for now - only a few uses, I make these decisions not only on "what is expected", but also on data, ie. "how this is usually used"

@georgehristov
Copy link
Collaborator

I really don't like the direction of stripping down the versatility...

@mvorisek
Copy link
Member Author

I really don't like the direction of stripping down the versatility...

I may seem so - but compare how many uses are "correct/canonical" and how many are like to be fixed - it is more than 100:1. Without the data, I would say probably the say the same as you...

Let's get this merged.

If you prefer array access like model access, getting values by field objects, you made extend the data Model and use it within your projects as you like.

But in lib, we need more standardized types and workflows, so everyone can understand it and the hidden bugs are discovered (for ex. mixed types of "current_row" and more than 10 others issues).

src/Table.php Outdated Show resolved Hide resolved
src/FormLayout/_Abstract.php Outdated Show resolved Hide resolved
src/FormLayout/_Abstract.php Outdated Show resolved Hide resolved
@atk4 atk4 deleted a comment from codecov bot Jun 12, 2020
@mvorisek mvorisek merged commit fe8948a into atk4:develop Jun 12, 2020
@mvorisek mvorisek deleted the refactor_hasxxx_to_trygetxxx branch June 12, 2020 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants