Skip to content

fix: add ArrayAccess to Session class #7

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

Merged
merged 3 commits into from
Dec 23, 2022

Conversation

yani
Copy link
Contributor

@yani yani commented Dec 21, 2022

This PR implements \ArrayAccess (#4)

The class now also triggers an error on retrieval of non-existing properties and array keys. This is consistent with original PHP behaviour. Retrieval still returns null when the error is triggered as this is also original behaviour. I had to use E_USER_WARNING instead of E_WARNING though. Would it be wise to change them to Exceptions?

I'm unsure where to add decent testing. I didn't find any tests for testing properties themselves. Should it go into PersistenceContext or a new test file?

@yani
Copy link
Contributor Author

yani commented Dec 21, 2022

I looked into the tests but I have no experience with behat. I'm unsure if I could write tests for this without spending a lot of time learning it.

@compwright
Copy link
Owner

@yani please rebase off the latest master branch and resolve conflicts. You'll see I've added features/access.feature and tests/behavior/AccessContext.php. You'll want to modify each test case and add a corresponding method as follows:

  Scenario: Check data does not exist
    When data does not exist
    Then property check returns false
    And array check returns false

Simply add And array... to each case. Then add a corresponding method to tests/behavior/AccessContext.php:

    /**
     * @Then array check returns false
     */
    public function arrayCheckReturnsFalse(): void
    {
        Assert::assertFalse(isset($this->session['foo']));
    }

@yani
Copy link
Contributor Author

yani commented Dec 22, 2022

Will do. Thanks

@yani
Copy link
Contributor Author

yani commented Dec 23, 2022

Updated:

  • array access tests
  • removed custom errors

@yani yani changed the title add ArrayAccess to Session class + throw error on invalid properties add ArrayAccess to Session class Dec 23, 2022
@compwright compwright changed the title add ArrayAccess to Session class Add ArrayAccess to Session class Dec 23, 2022
@compwright compwright changed the title Add ArrayAccess to Session class feat: add ArrayAccess to Session class Dec 23, 2022
@compwright compwright changed the title feat: add ArrayAccess to Session class fix: add ArrayAccess to Session class Dec 23, 2022
@compwright compwright merged commit 7d13b9d into compwright:master Dec 23, 2022
@yani yani deleted the arrayaccess branch December 24, 2022 05:41
compwright pushed a commit that referenced this pull request Dec 29, 2022
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.

2 participants