-
Notifications
You must be signed in to change notification settings - Fork 0
Demo/pr review demo #6
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
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,131 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
|
||
namespace DemoApp.Services | ||
{ | ||
// Wrong naming - should be PascalCase | ||
public class userService | ||
{ | ||
// Public field instead of property | ||
public string username; | ||
|
||
// Missing m_ prefix for private field | ||
private int userId; | ||
|
||
// No #region blocks for organization | ||
|
||
public string connectionString = "Server=localhost;Database=demo;User=sa;Password=admin123"; | ||
|
||
// Inconsistent parameter casing | ||
public async Task<string> GetData(string InputParam) | ||
{ | ||
// Wrong casing for local variable | ||
string Result = ""; | ||
|
||
// SQL Injection vulnerability | ||
var query = "SELECT * FROM Users WHERE Name = '" + InputParam + "'"; | ||
|
||
// Hardcoded password | ||
var password = "admin123"; | ||
|
||
// Not using async properly | ||
var users = GetAllUsers().Result; | ||
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. |
||
|
||
// String concatenation in loop | ||
for (int i = 0; i < users.Count; i++) | ||
{ | ||
Result = Result + users[i].Name + ", "; | ||
} | ||
|
||
// This method is way too long (imagine 50+ more lines here) | ||
// Violating the 30-line rule | ||
|
||
// Deep nesting example | ||
if (InputParam != null) | ||
{ | ||
if (InputParam.Length > 0) | ||
{ | ||
if (InputParam.Contains("admin")) | ||
{ | ||
if (password == "admin123") | ||
{ | ||
// Too deeply nested | ||
return "Admin access granted"; | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Magic number without constant | ||
if (users.Count > 100) | ||
{ | ||
return "Too many users"; | ||
} | ||
|
||
// Missing null checks | ||
var firstUser = users.First(); | ||
|
||
// Not disposing resources | ||
var connection = new System.Data.SqlClient.SqlConnection(connectionString); | ||
connection.Open(); | ||
// Never closed or disposed | ||
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. Bug: Unhandled Exceptions and Resource LeaksThe |
||
|
||
// Missing XML documentation | ||
// Missing error handling | ||
// Missing parameter validation | ||
|
||
return Result; | ||
} | ||
|
||
// Method name should be PascalCase | ||
public void processOrder(int OrderId) | ||
{ | ||
// Local variable should be camelCase | ||
var OrderStatus = "Processing"; | ||
|
||
// Public method without any documentation | ||
} | ||
|
||
// Missing async on method that returns Task | ||
public Task<List<User>> GetAllUsers() | ||
{ | ||
// Synchronous operation returning Task | ||
var users = new List<User>(); | ||
return Task.FromResult(users); | ||
} | ||
|
||
// N+1 query pattern example | ||
public void LoadUserOrders() | ||
{ | ||
var users = GetAllUsers().Result; | ||
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. |
||
|
||
foreach (var user in users) | ||
{ | ||
// This would execute a query for each user | ||
var orders = GetOrdersForUser(user.Id); | ||
user.Orders = orders; | ||
} | ||
} | ||
|
||
private List<Order> GetOrdersForUser(int userId) | ||
{ | ||
// Imagine database query here | ||
return new List<Order>(); | ||
} | ||
} | ||
|
||
// Helper classes | ||
public class User | ||
{ | ||
public int Id { get; set; } | ||
public string Name { get; set; } | ||
public List<Order> Orders { get; set; } | ||
} | ||
|
||
public class Order | ||
{ | ||
public int Id { get; set; } | ||
public string Status { get; set; } | ||
} |
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.
Bug: Credentials Hardcoded in Source Code
The database connection string, including sensitive credentials, is hardcoded directly in
userService.cs
. This exposes confidential information and creates a security vulnerability.