Hot answers tagged

13 votes

C# Am I using proper SOLID principles?

I suppose the simplest thing is to go through the principles: SRP - Cannot see any 'too big' things in your example OCP - The example is small enough and you are not using class inheritance that I ...
Jamie Stevenson's user avatar
8 votes
Accepted

C# Convert BLOB to jpg, resize, then back to BLOB

Welcome to CodeReview! I hope you get some good feedback. Absolute paths ...
Reinderien's user avatar
  • 59.1k
7 votes
Accepted

Audit table queries for various combinations of search criteria

First of all you should remove duplication. It's clearly that every if..else block has same code for filling DataTable and binding it to ListView. Move it to the bottom: ...
Sergey Berezovskiy's user avatar
7 votes

Combating magic strings in a web application

First point, these extensions are really cool. I don't have much comments to give, your code looks pretty good! The ActionInfo class could be simplified a little :...
IEatBagels's user avatar
  • 12.4k
7 votes
Accepted

ASP controller for tracking equipment

Index It would appear you primarily do three things here. Sort, Filter, and Page. There's not much to say about your paging, but the sorting and filtering logic clutter the action. Try creating an ...
Shelby115's user avatar
  • 1,961
6 votes
Accepted

External application for the web application deployment

static void Main(string[] args) If you're not going to use args, you can omit them: ...
svick's user avatar
  • 24.2k
6 votes
Accepted

Which is correct : Open one Connection for inserting List of objects or open connection for every single insertion?

In general, the best is to do all your inserts in one connection. Open/Close a connection implies a penalty. Although it is mitigated by some connection pooling mechanism on the database side (btw ...
Larry's user avatar
  • 266
6 votes

Factory for report handlers

Not sure if that's going to improve the maintainability index, but You're misimplementing Inversion of Control. IoC is when your dependency (namely, IReportHandler)...
guillaume31's user avatar
6 votes
Accepted

Update SQL database with using statement and SqlParameters

Stacking the using statements I am particularly difficult to please with code that has nesting, and I highly prefer to stack the ...
Svek's user avatar
  • 1,013
6 votes
Accepted

Implementing integration tests in ASP.NET core using InMemory database

I went through the very same conflict and eventually looked at what other frameworks were doing. What you'll find with the above implementation is that it is okay for a couple hundred tests, but soon ...
jaredcnance's user avatar
6 votes

Find consecutive numbers and turn them into ranges

Letting a single method do everything is not such a good idea. You should rather separate the logic for finding consecutive numbers from creating a string from ranges and let them produce the final ...
t3chb0t's user avatar
  • 44.2k
6 votes

C# Am I using proper SOLID principles?

IMO returning IEnumerable<T> from an API is a violation of "L", as running GetEnumerator() a second time (or ...
David Browne - Microsoft's user avatar
6 votes
Accepted

Is this correct way to setup DB Infrastructure with repository pattern and Dapper

Quick remarks: Why do you convert a string to a string: ...
BCdotWEB's user avatar
  • 11.2k
5 votes

Checking overlap ranges between two prices

I am adding my comments at first glance. This is not a final review as this code needs lots of refactoring. Delete commented code Never keep commented code; it adds distraction while reading by ...
Paritosh's user avatar
  • 2,638
5 votes

Recurring tasks in ASP.NET MVC

As already mentioned by Anders, the second approach is better, however there are several problems when implementing async Tasks that run within an ...
Alexei's user avatar
  • 1,786
5 votes

Factory for report handlers

Some observations: You shouldn't be creating the factory inside the class that uses it. That defeats its purpose! Instead, you should pass the factory to that class's constructor. You shouldn't be ...
Matthew Watson's user avatar
5 votes
Accepted

Online examination portal for learning

Overall, I think this code is a great first try. That said, it has a lot of room for improvement in terms of simplification and improving readability. As an aside, I'm not sure if you have a strong ...
Matthew FitzGerald-Chamberlain's user avatar
5 votes

If and else without curly brackets

Is it possible to have an if condition with curly brackets and else conditions without brackets? Or course it's possible. You don't even need new lines and can write everthing like this: ...
t3chb0t's user avatar
  • 44.2k
4 votes
Accepted

Refactored to use polymorphism in place of case statements

...
Heslacher's user avatar
  • 50.3k
4 votes

Validating users with Roles

...
Heslacher's user avatar
  • 50.3k
4 votes

Simple and reusable system for user registration and tracking and auto-updates - follow-up

if (decodedHeader.Equals(BasicAuthenticationKey)) { return true; } return false; Whenever you've got something like this, you can just directly return the ...
Pimgd's user avatar
  • 22.3k
4 votes

Identity OAuth implementation with Owin

A static class named OwinHelper, with methods SignIn, CreateIdentity and ...
Mathieu Guindon's user avatar
4 votes
Accepted

Safe Login Standards

Your password hash sucks. Password hashes need to be salted and slow. Good choices are PBKDF2, bcrypt, scrypt and Argon. See How to securely hash passwords? for details. ...
CodesInChaos's user avatar
  • 2,680
4 votes

Which is correct : Open one Connection for inserting List of objects or open connection for every single insertion?

Why do you employ a StringBuilder when you don't do anything with it? Especially when you should be using it. Right now the names of the various fields are defined ...
BCdotWEB's user avatar
  • 11.2k
4 votes
Accepted

Copy Data from Informix to SQL Server

@BCdotWEB gave you some constructive criticism about how to restructure this code to help break a giant operation into smaller, more manageable pieces. I strongly suggest you do that regardless of ...
James Fegan's user avatar
4 votes

ASP.NET Product Listing

I would definitely remove the database access from the Category partial view and move it to the Controller (you can add it to the view model or have a separate action for returning them which is even ...
Stefano d'Antonio's user avatar
4 votes

Code Behind to manage AD users for a CRM system

Do NOT use Hungarian notation: bAccionOk, sPassword,... Do NOT use non-alphanumeric characters: ...
BCdotWEB's user avatar
  • 11.2k
4 votes
Accepted

Using Identity to create user administration page

Let's take about the ManageViewModel for a moment, because it has a lot of things you can do better. ...
Der Kommissar's user avatar
4 votes
Accepted

Filtering ASP.NET membership roles without the default attributes

public abstract class BaseController : Controller { public MasterDbContext Context { get; } = new MasterDbContext(); } I really don't like this. Every single ...
Mathieu Guindon's user avatar

Only top scored, non community-wiki answers of a minimum length are eligible