-
Notifications
You must be signed in to change notification settings - Fork 1
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
Upgrade to simple cache 3 #4
Conversation
…ct typing to `DatabaseCache` and `DatabaseCacheIntegrationTest`
@@ -1,5 +1,7 @@ | |||
<?php | |||
|
|||
declare(strict_types=1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised - if you want strict type checking you have to specify it in every file - see https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.strict
I have the feeling that there is a tacit convention not to use this declaration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't "have to" specify it in every file, you specify it where you want strict types (no implicit typecasting / coercing of types).
It's true that for Kodus projects, we have generally not used these declarations, but there is no convention of ours that states we can't use it, if it makes sense.
On the PR for updating kodus/file-cache I suggest an alternative approach, that we could consider instead:
I'm actually wondering if we are doing a bit of overkill by asserting the TypeError being thrown. If for some reason we would forget the typehints on the parameters, then as long as we implement the interface correctly and adhere to Liskov's Substitution Principle, I think our implementation could be considered correct. In other words, if our implementation can handle an integer key, that doesn't conflict with the interface, as long as it can handle the string keys required by the interface...
Let's revisit this on tomorrows stand up :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1,13 +1,16 @@ | |||
<?php | |||
|
|||
declare(strict_types=1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…rade-simple-cache-3
DatabaseCache
based on interface changes