-
Notifications
You must be signed in to change notification settings - Fork 71
Post project completion merge request - ECommerce API #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6237a4f
187d5c9
c4f807b
e2ab51e
83ee73c
67f56b0
86e669d
e5cd1c0
d6dfc0a
9d1a617
21f7056
36e2f38
960eae8
d10cdbb
83633da
a9010d2
2f17a78
4ca62e7
33f27d0
1906d21
9c6ea8b
9d25885
b10df84
cc565fd
32f5b8f
65226b6
59fe6a7
a0f5e64
98f1e19
634aa74
8db809b
736ccc6
9c1113c
df39166
97483f3
855fbf3
f577e56
287cab7
e999b0d
666c1f0
090df33
5ac3221
27e7581
e7dedff
ce681f1
3225fd6
11a2e69
f008877
11ee02c
5e20934
ad868eb
60712e1
10bfd34
2d506ee
01360db
ba8f483
315fb77
d9282c9
5080b8b
4daf0b8
0a3abb4
6d246d4
2006e78
205a710
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||||||||||||
| using ECommerceApi.JJHH17.Models; | ||||||||||||||
| using ECommerceApi.JJHH17.Services; | ||||||||||||||
| using Microsoft.AspNetCore.Mvc; | ||||||||||||||
|
|
||||||||||||||
| namespace ECommerceApi.JJHH17.Controllers | ||||||||||||||
| { | ||||||||||||||
| [ApiController] | ||||||||||||||
| [Route("api/[controller]")] | ||||||||||||||
| // Example call: http://localhost:5609/api/category/ | ||||||||||||||
| public class CategoriesController : ControllerBase | ||||||||||||||
| { | ||||||||||||||
| private readonly ICategoryService _categoryService; | ||||||||||||||
|
|
||||||||||||||
| public CategoriesController(ICategoryService categoryService) | ||||||||||||||
| { | ||||||||||||||
| _categoryService = categoryService; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [HttpGet] | ||||||||||||||
| public ActionResult<PagedResponse<CategoryWithProductsDto>> GetAllCategories([FromQuery] Pagination pagination) | ||||||||||||||
| { | ||||||||||||||
| if (pagination.PageNumber < 1) pagination.PageNumber = 1; | ||||||||||||||
| if (pagination.PageSize < 1) pagination.PageSize = 10; | ||||||||||||||
|
|
||||||||||||||
| var allCategories = _categoryService.GetAllCategories(); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Inefficient 💡 Great job implementing pagination. But ideally you would want to pass the pagination responsibility down to your db context. 💭 Say your database has 10m rows. Currently your api would get 10m rows from the database, hold in memory and then filter down to the first 10 rows (as an example). Imagine the cost, time, and resource implications of this. If using a cloud database provider you would quickly run out of money. 🔧 Instead you should use the database to only get the 10 rows you need, only hold 10 rows in memory, etc, etc. |
||||||||||||||
|
|
||||||||||||||
| var totalRecords = allCategories.Count; | ||||||||||||||
| var pagedData = allCategories | ||||||||||||||
| .OrderBy(c => c.CategoryId) | ||||||||||||||
| .Skip((pagination.PageNumber - 1) * pagination.PageSize) | ||||||||||||||
| .Take(pagination.PageSize) | ||||||||||||||
| .ToList(); | ||||||||||||||
|
|
||||||||||||||
| return Ok(pagedData); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [HttpGet("{id}")] | ||||||||||||||
| public ActionResult<Category> GetCategoryById(int id) | ||||||||||||||
| { | ||||||||||||||
| var result = _categoryService.GetCategoryById(id); | ||||||||||||||
|
|
||||||||||||||
| if (result == null) { return NotFound(); } | ||||||||||||||
|
|
||||||||||||||
| return Ok(result); | ||||||||||||||
|
Comment on lines
+42
to
+44
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Simplify 💡 Use the ternary conditional operator. 💭 e.g.
Suggested change
|
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [HttpPost] | ||||||||||||||
| public ActionResult<CreateCategoryDto> CreateCategory(CreateCategoryDto category) | ||||||||||||||
| { | ||||||||||||||
| return Ok(_categoryService.CreateCategory(category)); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 REST 💡 Post requests should return a 201 Created response to indicate successful creation and the location of the created resource. |
||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,52 @@ | ||||||||||
| using ECommerceApi.JJHH17.Models; | ||||||||||
| using ECommerceApi.JJHH17.Services; | ||||||||||
| using Microsoft.AspNetCore.Mvc; | ||||||||||
|
|
||||||||||
| namespace ECommerceApi.JJHH17.Controllers | ||||||||||
| { | ||||||||||
|
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| [ApiController] | ||||||||||
| [Route("api/[controller]")] | ||||||||||
| // Example call: http://localhost:5609/api/product/ | ||||||||||
| public class ProductsController : ControllerBase | ||||||||||
| { | ||||||||||
| private readonly IProductService _productService; | ||||||||||
| public ProductsController(IProductService productService) | ||||||||||
| { | ||||||||||
| _productService = productService; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| [HttpGet] | ||||||||||
| public ActionResult<PagedResponse<GetProductsDto>> GetAllProducts([FromQuery] Pagination pagination) | ||||||||||
| { | ||||||||||
| if (pagination.PageNumber < 1) pagination.PageNumber = 1; | ||||||||||
| if (pagination.PageSize < 1) pagination.PageSize = 10; | ||||||||||
|
|
||||||||||
| var allProducts = _productService.GetAllProducts(); | ||||||||||
|
|
||||||||||
| var totalRecords = allProducts.Count; | ||||||||||
| var pagedData = allProducts | ||||||||||
| .OrderBy(c => c.productId) | ||||||||||
| .Skip((pagination.PageNumber - 1) * pagination.PageSize) | ||||||||||
| .Take(pagination.PageSize) | ||||||||||
| .ToList(); | ||||||||||
|
|
||||||||||
| return Ok(pagedData); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| [HttpGet("{id}")] | ||||||||||
| public ActionResult<Product> GetProductById(int id) | ||||||||||
| { | ||||||||||
| var result = _productService.GetProductById(id); | ||||||||||
|
|
||||||||||
| if (result == null) { return NotFound(); } | ||||||||||
|
|
||||||||||
| return Ok(result); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| [HttpPost] | ||||||||||
| public ActionResult<CreateProductDto> CreateProduct(CreateProductDto product) | ||||||||||
| { | ||||||||||
| return Ok(_productService.CreateProduct(product)); | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 CreatedAt |
||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||
| using ECommerceApi.JJHH17.Models; | ||||||||||
| using ECommerceApi.JJHH17.Services; | ||||||||||
| using Microsoft.AspNetCore.Mvc; | ||||||||||
|
|
||||||||||
| namespace ECommerceApi.JJHH17.Controllers | ||||||||||
| { | ||||||||||
|
Comment on lines
+5
to
+6
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| [ApiController] | ||||||||||
| [Route("api/[controller]")] | ||||||||||
| // Example call: http://localhost:5609/api/sale/ | ||||||||||
| public class SalesController : ControllerBase | ||||||||||
| { | ||||||||||
| private readonly ISaleService _saleService; | ||||||||||
| public SalesController(ISaleService saleService) | ||||||||||
| { | ||||||||||
| _saleService = saleService; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| [HttpGet] | ||||||||||
| public ActionResult<PagedResponse<GetProductsDto>> GetAllSales([FromQuery] Pagination pagination) | ||||||||||
| { | ||||||||||
| if (pagination.PageNumber < 1) pagination.PageNumber = 1; | ||||||||||
| if (pagination.PageSize < 1) pagination.PageSize = 10; | ||||||||||
|
|
||||||||||
| var allSales = _saleService.GetAllSales(); | ||||||||||
|
|
||||||||||
| var totalRecords = allSales.Count; | ||||||||||
| var pagedData = allSales | ||||||||||
| .OrderBy(c => c.SaleId) | ||||||||||
| .Skip((pagination.PageNumber - 1) * pagination.PageSize) | ||||||||||
| .Take(pagination.PageSize) | ||||||||||
| .ToList(); | ||||||||||
|
|
||||||||||
| return Ok(pagedData); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| [HttpGet("{id}")] | ||||||||||
| public ActionResult<Sale> GetSaleById(int id) | ||||||||||
| { | ||||||||||
| var result = _saleService.GetSaleById(id); | ||||||||||
|
|
||||||||||
| if (result == null) { return NotFound(); } | ||||||||||
|
|
||||||||||
| return Ok(result); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| [HttpPost] | ||||||||||
| public ActionResult<SaleWithProductsDto> CreateSale([FromBody] CreateSaleDto sale) | ||||||||||
| { | ||||||||||
| try | ||||||||||
| { | ||||||||||
| var created = _saleService.CreateSale(sale); | ||||||||||
|
|
||||||||||
| var dto = new SaleWithProductsDto( | ||||||||||
| created.SaleId, | ||||||||||
| created.ItemCount, | ||||||||||
| created.SalePrice, | ||||||||||
| created.Products | ||||||||||
| .OrderBy(p => p.ProductName) | ||||||||||
| .Select(p => new ProductDto(p.ProductId, p.ProductName, p.Price)) | ||||||||||
| .ToList() | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| return CreatedAtAction(nameof(GetSaleById), new { id = created.SaleId }, dto); | ||||||||||
| } | ||||||||||
| catch (ArgumentNullException e) | ||||||||||
| { | ||||||||||
| return BadRequest(new { error = e.Message }); | ||||||||||
| } | ||||||||||
| catch (ArgumentException ex) | ||||||||||
| { | ||||||||||
| return BadRequest(new { error = ex.Message }); | ||||||||||
| } | ||||||||||
| catch (InvalidOperationException ex) | ||||||||||
| { | ||||||||||
| return NotFound(new { error = ex.Message }); | ||||||||||
| } | ||||||||||
|
Comment on lines
+65
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Exception Handling 💡 The only method in your API controllers with a try catch block, and some weird exception handling. Seems like a bit of a code smell. Is there another more elegant way you could have handled these edge-cases? |
||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Namespaces
💡 As per the academy's code-conventions, use file-scoped namespaces.
➡️ From C# 10, we are able to remove the block/braces and save on level of indentation, optimising space and improving readability.