Comments (15)
Then, for consistency need do the same with all methods in all classes (many of them at the same time getter and setters).
from core.
This is a pattern built in Kohana from the very beginning. I think it's very elegant and should stay this way.
How would it permit greater flexibility?
from core.
For example, I want to extend base class and use second parameter in getter. It will not work, because setter using second parameter.
from core.
You mean like using a default value?
If you extend the Request class, $_get, $_post, $_cookies are all defined protected.
from core.
A method shouldn't have more than one responsibility and it should have an obvious/clear name relating to its purpose. Using "get_cookie()" and "set_cookie()" makes for a lot more cleaner code than using "cookie()" everywhere.
Splitting methods up to ensure they have a single responsibility is a good thing to do throughout the whole of the Kohana code base.
from core.
I agree, also the single getter/setter leads to a lot of boilerplate logic all over the place, has side effects (as currently implemented) such as not being able to set NULL, and plays havoc with IDEs because of every method having two possible return types.
This will be a big change, breaking a lot of third party modules as well as end-user code. Perhaps we could introduce new getter/setter methods as proxies and mark the old-style ones deprecated for this major release series?
from core.
+1 for separated methods. But this is just one more backwards-incompatible change purposed for Kohana.
from core.
The old methods can easily become aliases and deprecated so that all code which uses Kohana will continue to work until we properly remove them in the major version 4.0.
from core.
@lenton the new methods should be the aliases, otherwise existing code that extends setters/getters will break.
from core.
@acoulton How would it? When extending a method you replace all the code so surely it wouldn't matter what the existing code is?
from core.
@lenton suppose people have custom logic in the getter/setter (struggling to think of a valid reason why, but doesn't mean they won't have).
If we make get_query
call query
then that logic will still be called whichever getter you use.
If we make query
call get_query
then only calls to query
will be calling the overridden method.
from core.
@acoulton Ah ok, I see.
from core.
The getter/setter usually covers 4 responsabilities:
- assigning an array
- retrieving an array
- assigning a value to a key
- retrieving a value from a key
How would all these be splitted?
This is and complexify considerably the framework code and go against Kohana spirit. I really fear that we move toward some Java-like approach and end-up with distinct getter and setter all over the framework. Let's say I really like the simplicity and elegance the pattern provide.
@acoulton setting NULL is not particulary useful.
Also, I'm not a big fan of leaving that many methods in the framework deprecated.
from core.
@arteymix well probably if you were designing from scratch then for example Request would just have a getter that returns some kind of ArrayObject and then replacing the array, assigning a single key, getting as array and getting a single key would be separate methods on that object.
But since we're not designing from scratch, I'd say the array properties should be just split to a setter (pass an array or single key value) and a getter (return a single key or return all of them).
Let's say I really like the simplicity and elegance the pattern provide.
It's fine-ish when it's strings, but I don't think it's simple or elegant that Request->client() may return either a Request_Client or $this depending what you pass into it. Not least because IDEs and code analysis tools generally can't figure that out, so you have real issues with chained method calls unless you then add explicit typehints or warning suppressions all over your code.
@acoulton setting NULL is not particulary useful.
That very much depends. It's not commonly useful particularly on an empty class, but it can be required if you want to overwrite an existing value. More importantly, it can cause unexpected fatal errors - consider for example this code:
public function action_external()
{
$request = Request::factory('http://some.api')
->query('active_only', $this->request->post('active_only'))
->query('search', $this->request->post('search'))
->query('auth', 'token');
$this->response->body($request->execute());
}
Looks simple and elegant enough. Imagine active_only is a checkbox on your form - if so, and it's not checked, then $this->request->post('active_only')
will return NULL. That means that $this>query('active_only', $this->request->post('active_only'))
acts as a getter and returns NULL. And that makes the next line fatal.
Sure, if you know about that then you can protect against invalid input, but it's not a remotely obvious failure case from just looking at the code. I don't think that's simple or elegant.
Also, I'm not a big fan of leaving that many methods in the framework deprecated.
Me either, but I'm a bigger fan than of leaving that many methods in the framework for end-users to change themselves and of breaking every third-party module they may be using. So if we do this, it needs to be done by deprecation.
from core.
I'm aware of this and always provide default value where appliable.
I think it is a good idea to go forward though. The approach you will take on this will reflect all over the framework, so make a good decision.
Here are my thoughts:
- getter should always have a default value as a second argument defaulted to NULL
- getter must return the instance for builder syntax
- setter should always allow to set an array of value as first argument
- the change must be over all the framework and retrocompatible like you suggest
How long would the actual getter/setter be kept for retrocompatibility?
from core.
Related Issues (20)
- Use of mikey179/vfsStream for Log tests breaks module builds HOT 3
- 3.4.0 current status HOT 49
- Change detecting urls starting with //
- Improvements on website HOT 10
- 1 repo to rule them all HOT 12
- .git files in modules release for 3.3.5 HOT 4
- 4.0.0 release HOT 16
- Ubuntu packages HOT 9
- modules and composer - play together nicer HOT 2
- Should we remove 'action', 'controller', 'directory' from request params? HOT 4
- Implementation of external requests in Minion Task HOT 5
- [Security] Encrypt HOT 34
- Issues with PHP 7.0.6 and ORM HOT 3
- Use Route:url in Minion Task HOT 1
- ERROR: Kohana_Exception [ 0 ]: Directory APPPATH/cache must be writable HOT 8
- "content-length" -header calculation in Response
- Server Upgraded to PHP7 Error: __toString() must not throw an exception HOT 4
- Error en Core Handle
- Function Valid::date for timestamp HOT 1
- [Proposal] Make middlewares HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from core.