Conversation
okuzan
left a comment
okuzan
left a comment
There was a problem hiding this comment.
Separate your controllers into packages
| - name: Set up JDK 17 | ||
| uses: actions/setup-java@v2 | ||
| with: | ||
| java-version: '11' | ||
| java-version: '17' |
There was a problem hiding this comment.
I did not. that time i amended commit by mistake
| @Override | ||
| protected void doGet(HttpServletRequest req, HttpServletResponse resp) | ||
| throws ServletException, IOException { | ||
| driverService.delete(Long.valueOf(req.getParameter("id"))); |
There was a problem hiding this comment.
Redirect the user to another page after the operation. Currently, the user will be left on a blank page after a driver is deleted.
| manufacturer.setName(name); | ||
| manufacturer.setCountry(country); | ||
| manufacturerService.create(manufacturer); | ||
| } |
There was a problem hiding this comment.
You should add a redirect after creating a Manufacturer. Otherwise, the user will stay on the same page without any feedback.
| @Override | ||
| protected void doGet(HttpServletRequest req, HttpServletResponse resp) | ||
| throws ServletException, IOException { | ||
| manufacturerService.delete(Long.valueOf(req.getParameter("id"))); |
There was a problem hiding this comment.
After deleting a manufacturer, the user should be redirected to a specific page, usually the list of all manufacturers, to confirm the deletion.
| Long carId = Long.valueOf(req.getParameter("car_id")); | ||
| Driver driver = driverService.get(driverId); | ||
| Car car = carService.get(carId); | ||
| carService.addDriverToCar(driver, car); |
There was a problem hiding this comment.
After processing the POST request successfully, it is common practice to redirect the client to a specific URL, rather than leaving them on the current POST URL.
| driver.setName(name); | ||
| driver.setLicenseNumber(licenseNumber); | ||
| driverService.create(driver); | ||
| } |
There was a problem hiding this comment.
After executing the 'create' method, the user is not redirected to any page. This might cause confusion for the user as no visual feedback will be provided after submitting the form.
| Car car = new Car(); | ||
| String carModel = req.getParameter("car_model"); | ||
| Long manufacturerId = Long.valueOf(req.getParameter("manufacturer_id")); | ||
| Manufacturer manufacturer = manufacturerService.get(manufacturerId); | ||
| car.setManufacturer(manufacturer); | ||
| car.setModel(carModel); | ||
| car.setDrivers(new ArrayList<>()); | ||
| carService.create(car); |
There was a problem hiding this comment.
It's better to create variables when we plan to use them
| Car car = new Car(); | |
| String carModel = req.getParameter("car_model"); | |
| Long manufacturerId = Long.valueOf(req.getParameter("manufacturer_id")); | |
| Manufacturer manufacturer = manufacturerService.get(manufacturerId); | |
| car.setManufacturer(manufacturer); | |
| car.setModel(carModel); | |
| car.setDrivers(new ArrayList<>()); | |
| carService.create(car); | |
| String carModel = req.getParameter("car_model"); | |
| Long manufacturerId = Long.valueOf(req.getParameter("manufacturer_id")); | |
| Manufacturer manufacturer = manufacturerService.get(manufacturerId); | |
| Car car = new Car(); | |
| car.setManufacturer(manufacturer); | |
| car.setModel(carModel); | |
| car.setDrivers(new ArrayList<>()); | |
| carService.create(car); |
vaniii1
force-pushed
the
hw-web-practice
branch
2 times, most recently
from
baa9deb to
02717b1
Compare
| Long carId = Long.valueOf(req.getParameter("car_id")); | ||
| Driver driver = driverService.get(driverId); | ||
| Car car = carService.get(carId); | ||
| carService.addDriverToCar(driver, car); |
| driver.setName(name); | ||
| driver.setLicenseNumber(licenseNumber); | ||
| driverService.create(driver); | ||
| resp.sendRedirect("/drivers"); |
| - name: Set up JDK 17 | ||
| uses: actions/setup-java@v2 | ||
| with: | ||
| java-version: '11' | ||
| java-version: '17' |
5 participants
No description provided.