Setup changes #3
Conversation
| $this->model = $model; | ||
| } | ||
|
|
||
| public function __invoke(RequestInterface $request, ResponseInterface $response): ResponseInterface |
There was a problem hiding this comment.
This is one thing to think about.
The ResponseInterface does not include the withJson method, which means phpstorm gives some weird warnings when you use it.
We could fix this by changing the type-hint to Slim\Http\Response, but then it looks a bit weird having the response be an interface, but the request being an actual class.
Does anyone have any thoughts?
There was a problem hiding this comment.
it also breaks D in SOLID
it could be a really good teaching point, but it could also confuse them unnecessarily
we could use the class as the type hint for both, still breaks SOLID but is consistent atleast
There was a problem hiding this comment.
I have a better idea
slimphp/Slim-Http#194
src/Models/CoursesModel.php
Outdated
| public function __construct(PDO $db) | ||
| { | ||
| $this->db = $db; | ||
| } |
There was a problem hiding this comment.
Another thing to think about here - I've added this example model which takes a PDO connection, but the issue there is that students will need to connect slim to a DB straight away.
There was a problem hiding this comment.
having a modal with PDO doesnt feel like a skeleton anymore, I see the benefit for students and teaching, but as a usable skeleton app your gonna have to remove the modal and probably the controller every time.
There was a problem hiding this comment.
oh just realised the modal doesnt actually use the DB, so they only need to have a DB with he right name, which isn't too bad
There was a problem hiding this comment.
perhaps we could be sneaky and when we get them to do the SQL excercises in week 2 (adults/children etc..) we could make that named example. That way we could pre-emptively solve the prob before it arises?
app/dependencies.php
Outdated
| return $renderer; | ||
| }; | ||
| $container[LoggerInterface::class] = DI\factory(LoggerFactory::class); | ||
| $container['renderer'] = DI\factory(RendererFactory::class); |
There was a problem hiding this comment.
think we should normalise the use of ClassName::class for the DIC keys, make it consistent with the Logger
There was a problem hiding this comment.
yeah agreed, def a syntax for the students thats good to get comfortable with
|
|
||
| class CoursesAPIController | ||
| { | ||
| protected CoursesModel $model; |
| $this->model = $model; | ||
| } | ||
|
|
||
| public function __invoke(RequestInterface $request, ResponseInterface $response): ResponseInterface |
There was a problem hiding this comment.
it also breaks D in SOLID
it could be a really good teaching point, but it could also confuse them unnecessarily
we could use the class as the type hint for both, still breaks SOLID but is consistent atleast
src/Models/CoursesModel.php
Outdated
| public function __construct(PDO $db) | ||
| { | ||
| $this->db = $db; | ||
| } |
There was a problem hiding this comment.
having a modal with PDO doesnt feel like a skeleton anymore, I see the benefit for students and teaching, but as a usable skeleton app your gonna have to remove the modal and probably the controller every time.
src/Models/CoursesModel.php
Outdated
| public function __construct(PDO $db) | ||
| { | ||
| $this->db = $db; | ||
| } |
There was a problem hiding this comment.
oh just realised the modal doesnt actually use the DB, so they only need to have a DB with he right name, which isn't too bad
app/dependencies.php
Outdated
| return $renderer; | ||
| }; | ||
| $container[LoggerInterface::class] = DI\factory(LoggerFactory::class); | ||
| $container['renderer'] = DI\factory(RendererFactory::class); |
There was a problem hiding this comment.
yeah agreed, def a syntax for the students thats good to get comfortable with
src/Models/CoursesModel.php
Outdated
| public function __construct(PDO $db) | ||
| { | ||
| $this->db = $db; | ||
| } |
There was a problem hiding this comment.
perhaps we could be sneaky and when we get them to do the SQL excercises in week 2 (adults/children etc..) we could make that named example. That way we could pre-emptively solve the prob before it arises?
No description provided.