Aded Data Helper and Session Helper#12
Conversation
src/Data.php
Outdated
There was a problem hiding this comment.
we should avoid static whenever possible in Ohanzee.
There was a problem hiding this comment.
What do you prefer in that instance? const??
Context: I strayed away from 'const' originally because the data would be public, I guess it seems trivial for it not to be in this case.
There was a problem hiding this comment.
I also, notice that you are calling local methods using static::method(), do you prefer that to self::method(). Not sure if there's a need for static::method() in a no dependency library since there should be no late static bindings. Unless you foresee users extending these classes in such a way.
There was a problem hiding this comment.
replaced static props with constants where possible.
|
It would be really helpful if these new classes came with phpspec tests. |
|
Sounds good to me. I'll get some setup. |
|
Also, just reviewed the psr-2 spec and realized I have some solid changes to make. Should be up to spec in a day or two. |
src/Session.php
Outdated
There was a problem hiding this comment.
here, the expression already returns boolean, you don't need the ternary at all.
…id during active session
|
I think I've addressed all of your concerns from the last code review. I still need to get on some phpspec tests, though. |
|
@shadowhand it would be possible to merge this and make a ticket for tests. |
|
It should be composed of two commits, not 26. And it needs tests. |
3 participants
These are two objects I find myself using a lot. I feel like the Class Name "Data" might be a bit too generic and would advise an alternate name, nothing came to mind right away.