Audubon Project Commit #2
Conversation
|
|
||
| class BirdController extends Controller | ||
| { | ||
| public function audubonSociety(): view |
There was a problem hiding this comment.
Function names should be more descriptive of what they do. In this case you're adding a sighting, so sighting or addSighting would be a better function name.
|
|
||
| public function saveAudubonSociety(Request $request, saveService $saveService): view | ||
| { | ||
| //var_dump($request->input()); |
There was a problem hiding this comment.
var_dump that you forgot to remove.
| return view('redirect-after-submit'); | ||
| } | ||
|
|
||
| public function displayAllSightings(): view |
There was a problem hiding this comment.
This name is fine, but often for displaying multiple lists we often will use index as the function name that shows multiple entries of something. Not every time, but usually. I probably wouldn't call this out, but it's worth noting.
| private $id; | ||
| private $date_time; | ||
| private $location; | ||
| private $member_id; |
There was a problem hiding this comment.
In most cases, do not use _. So it should be $memberID. $date_time could be $date or something more descriptive if you want. Generally $date is pretty obvious what it is for. Like in this case it's safe to assume that date is the date of the capture, so I don't think it's necessary to be any more descriptive than $date.
|
|
||
| public function putDataIntoClasses($data): void | ||
| { | ||
| $this->birdObj = $this->getBirdData($data); |
There was a problem hiding this comment.
Unless you have a few bird related variables, I think obj is unnecessary. There may be some cases where you access it many times on various functions where it makes sense to make it a global variable. In this case, you'd only have to pass it to one function, so I'd just make it a local variable. $bird, $member, $capture. That is the syntax for creating a local variable.
| { | ||
| Schema::create('Member', function($table){ | ||
| $table->increments('id')->unsigned(); | ||
| $table->string('name', 50)->nullable(); |
There was a problem hiding this comment.
We're starting to use char now. string creates varchar.
| Schema::create('Bird', function($table){ | ||
| $table->increments('id')->unsigned(); | ||
| $table->string('species', 50); | ||
| $table->string('description', 100); |
There was a problem hiding this comment.
Description is very likely to have cases where it's pretty long. It should be a text. Also, you used a text input on the form, but it should be textarea to give more room to type.
| <head> | ||
| <title>Audubon Society Capture Form</title> | ||
| </head> | ||
| <body style='text-align: center; border-width:5px; border-style:solid; color:black; width:fit-content'> |
There was a problem hiding this comment.
It's fine to add styles in this test project, but very rarely should we add inline styles. style="display: none" is commonly used if we want to hide by default. That inline style is fine, but others shouldn't be used for the most part.
| use Audubon\Entities\Member; | ||
| use PHPUnit\Framework\TestCase; | ||
|
|
||
| final class saveServiceTest extends TestCase |
There was a problem hiding this comment.
classes should always start with caps. So it should be SaveService and SaveServiceTest.
Audubon Project:
Bird, Member, Capture Entities and Database Tables
BirdController Controller
Migration File For Creating Database and Tables
Form View, Thanks for the Submission View, All Sightings View
Save service class
Created unit tests for service class