Adding dirty code to test code review#149
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces debugging code and formatting changes to the RegionRoutes.cs file. Based on the title, this appears to be intentionally "dirty" code added for testing code review functionality.
Key changes include:
- Addition of debugging/testing code that hardcodes values
- Formatting modifications that affect code readability
- Logic changes that may break normal API functionality
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| id = 1; //testing with item 1 | ||
|
|
||
| var region = await db.Regions | ||
| .SingleOrDefaultAsync(p => p.Id == id); | ||
|
|
There was a problem hiding this comment.
This hardcoded assignment overwrites the input parameter, making the endpoint always return the region with ID 1 regardless of the requested ID. This breaks the API contract and should be removed.
| id = 1; //testing with item 1 | |
| var region = await db.Regions | |
| .SingleOrDefaultAsync(p => p.Id == id); | |
| var region = await db.Regions | |
| .SingleOrDefaultAsync(p => p.Id == id); |
| return | ||
|
|
||
| Results.NotFound(); |
There was a problem hiding this comment.
Unnecessary line breaks in the return statement reduce code readability. The return statement should be on a single line: return Results.NotFound();
| return | |
| Results.NotFound(); | |
| return Results.NotFound(); |
|
|
||
| var region = await db.Regions.Include(p => p.Departments) | ||
| .SingleOrDefaultAsync(p => p.Id == id); | ||
| .SingleOrDefaultAsync(p => p.Id == 0); |
There was a problem hiding this comment.
Hardcoded comparison to 0 instead of the id parameter means this query will never find the requested region. This should be p => p.Id == id to match the input parameter.
| .SingleOrDefaultAsync(p => p.Id == 0); | |
| .SingleOrDefaultAsync(p => p.Id == id); |
| if (region is not null) | ||
| { | ||
| if (db is not null) | ||
| { | ||
| return Results.BadRequest(RequestMessages.BadRequest); | ||
| } | ||
| } |
There was a problem hiding this comment.
These nested null checks are unnecessary and add complexity. At this point in the code, both region and db are guaranteed to be non-null. The original simple return statement should be restored.
| if (region is not null) | |
| { | |
| if (db is not null) | |
| { | |
| return Results.BadRequest(RequestMessages.BadRequest); | |
| } | |
| } | |
| return Results.BadRequest(RequestMessages.BadRequest); |
| } | ||
| } | ||
|
|
||
| var listDeparments = region.Departments = queryDepartments.ToList(); |
There was a problem hiding this comment.
This assignment modifies the region.Departments property with the sorted list, which could have unintended side effects. The variable should be assigned without modifying the region object: var listDeparments = queryDepartments.ToList();
| var listDeparments = region.Departments = queryDepartments.ToList(); | |
| var listDeparments = queryDepartments.ToList(); |
| } | ||
| } | ||
|
|
||
| var listDeparments = region.Departments = queryDepartments.ToList(); |
There was a problem hiding this comment.
Variable name listDeparments is misspelled. It should be listDepartments.
No description provided.