24
Why bother checking if the value/property has been changed? You are just adding additional complexity for no reason. Sure, it might be slightly! faster to only update modified values, but in return you are adding an overhead to keep track of the modified values, which will diminish whatever performance boost you got.
Just invoke Update on the entire object....
17
Have a look on the Onion Architecture series by Jeffrey Palermo
How about this for a structure
Project.Domain (Class Library)
has no deppendencies on any other projects
represents the core business of your app/service
any external dependecies are abstracted away via DI (adapter pattern)
you might want to define an IDataContext/ISession that abstracts ...
15
As you have already mentioned, DbContext is a UoW, and DbSet is a repository -- there is no need to reimplement those patterns, unless you're into ridiculously useless complexity.
Entity Framework wraps all pending changes in a transaction for you already, so each DbContext in an application contains DbSets that are somehow related. Blogs and Posts are ...
14
Use UriBuilder and set Port to -1
Uri oldUri = new Uri("http://myhost:443/Home/Index");
UriBuilder builder = new UriBuilder(oldUri);
builder.Port = -1;
Uri newUri = builder.Uri;
In case you want to remove the port part only when it is default, e.g. 80 for http and 443 for https, use snippet below (credit goes to Chris for the idea)
static Uri ...
14
You can make DbContext protected property in your base implementation, so you don't have to declare it in every single derived class.
You might want to consider extracting Insert and Delete methods to separate interface, because often you want to make some of your repositories readonly.
Implementation may look like that:
interface IRepository<T>
{...
11
I would change your method to use generics (so that the return value is meaningful) and not call .ToString() internally. This will enable you to use it in more scenarios.
public static T GetValueTernary<T>(this HtmlHelper html, object a, object b, T valueIfEqual, T valueIfNotEqual)
{
return object.Equals(a, b) ? valueIfEqual : valueIfNotEqual;
}
...
10
Your code will throw an exception when a is null. A safe way to compare values is the static version of object.Equals():
object.Equals(a, b)
10
Don't use underscores in identifiers. If that's how the database has them, use a ColumnAttribute to specify the column name scratch that, you're going code-first. This:
public int Tournament_Id { get; set; }
public int Match_Id { get; set; }
public int Team_Id { get; set; }
public int PlayerAccount_Id { get; set; }
Is much more seesharpesque like this:
...
10
Let me know if I went wrong somewhere. This is the first time I set up an application with a proper architecture.
I'm afraid there is no such thing as general "proper" architecture. Relevant architecture is the one that enables/assists developers in implementing new functionality or adjusting your solution to new requirements.
In your implementation I don'...
9
I made a small example to show you that all your worries are in the past. Before you implement this yourself though, keep Mat's remarks in mind: I made this as a quick sketch and separating the mappings from the context, naming conventions, etc are important for your code's clarity.
That being said, this small setup shows you how you can change it:
public ...
8
I have witnessed the UriBuilder .port = -1 fix elsewhere. Frankly guys, it is a hack that really isn't a good production ready solution.
The right solution here is to use UriBuilder as follows.
var uri = new UriBuilder(baseUri);
string newUrl = uri.Uri.ToString();
8
I'll answer your second question first! The fact that one of your your checkboxes needs to be checked is a validation that is more about your class than one of your checkbox. I mean, individually, a checkbox doesn't need to be checked, but your model requires one of them to be. Which is why I would put the validation attribute on your class instead of on one ...
8
In C# 6 or later you can write it in a single line of code using the null conditional operator (?.) and the null coalescing operator (??):
ver errorMessage = exception.InnerException?.Message ??
exception.Message ??
exception.ToString();
Note that it will not handle empty strings (if the Message property contains ...
7
This switch should be an if statement instead.
if (input == null) // we didn't get an input from GetCustomEditorInput
{
switch (property.PropertyType.Name)
{
case "Boolean":
input = new ExpressFormsCheckBox()
{
FormName = FormName,
...
7
It sounds like you need to add a service layer where you can include all your business logic. This way your controller classes do not become bloated with business logic and they are only dealing with the handling of requests and the population of view-models.
Using thin controllers like this you can separate your logic out to different services and make it ...
7
I added some formatting to that nested ternery to try to understand what it's doing, and it still took me a while!
model.Username = (db.Users.Any(p => p.UserName == model.Username))
? model.Username
: (db.Users.Any(p => p.Email == model.Username))
? db.Users.SingleOrDefault(p => p.Email == model.Username).UserName
: ...
7
First of all, get rid of this line:
JsonResult json = new JsonResult();
Just use return new JSON(...).
Now, you have repeating chunks of codes when you construct the angles and points Lists. I recommend you extract them to separate methods. If you do not want to clutter your code with the methods use the delegate() or Func<>() to create a functions ...
6
I'm assuming this code hasn't been reviewed yet because it's... beautiful - and I'm still searching for a better word. I mean, wow that's clever, I want that!!
The only thing I can see here, is in the View method:
protected override ViewResult View(string viewName, string masterName, object model)
{
var result = base.View(viewName, masterName, model);
...
6
A simple option would be to use reflection to check for properties that are virtual and has the Id-suffix. This is where I came up with, working for me;
public static IQueryable<T> IncludeAll<T>(this IQueryable<T> queryable) where T : class
{
var type = typeof (T);
var properties = type.GetProperties();
foreach (var property in ...
6
How about something like this. It's pretty much what you have but all I have done is seperated a couple of the parts into methods
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Web;
using System.Web.Http;
using System.Web.Http.Controllers;
using System.Web.Security;
using WebMatrix.WebData;
using ...
6
You're getting there. Here's a pattern that I've used that works really well. I don't like generic "find" and "save" methods because they tend to be really complicated in order to work with everything and I find you end up pulling your hair out when you want to do something special.
Here's the base repository class. We want concrete implementations so we ...
6
If you are worried about the update statement that it might slow the query down, I would suggest a slightly different approach. You can modify your code to pass in the modified property list. So that in scenarios when you know which property is updated you have an option to make it even quicker and also when no property is supplied, then you can update all - ...
6
I suggest to use the same name for primary and foreign keys following some consistent pattern:
It could be e.g. _id or Id, so you will have in PlayerAccount table the primary key PlayerAccountId (instead of just Id) and in PlayerTeamKey also PlayerAccountId as a foreign key.
Naming of fields and classes is very important in non-small projects.
I believe ...
6
I know this is an old ticket, but I think I might have an approach that uses only one round trip to the database.
using (var db = new ApplicationDbContext())
{
string myPhone = String.Join("", model.UserName.Split('(', '-', ')'));
ApplicationUser myUser = (ApplicationUser)db.Users.FirstOrDefault(u =>
...
6
Do you know about radians?
Math.Cos (and the other trig functions in .Net) are based on radians not degrees.
Math.Cos(90) radians is roughly Math.Cos(5157) in degrees. Which is a bit odd...
There are 2π radians in a circle so 1 degree is π/180. As long as you use the same value the lines will be parallel anyway but I thought it was worth pointing out.
...
6
Assuming you have at least c# 6 (and thus, the null propagation operator), you can simplify it like this:
if (!string.IsNullOrEmpty(exception?.InnerException?.Message))
{
errorMessage = exception.InnerException.Message;
}
else if (!string.IsNullOrEmpty(exception?.Message))
{
errorMessage = ...
5
EF will not create a wrapper around the Restaurant class! Instead it will derive from your Restaurant class and it will override that property (public ICollection<RestaurantReview> Reviews) because it needs to create a proxy or navigation property instead of an everyday property. If you don't specify that property as a virtual one, the EF can not ...
5
At the very least, you should place the second if statement in a finally block. As things stand, the code is not generally correct from a purely mechanical perspective. I would also phrase it as:
bool dbDispose = db == null;
db = db ?? new DataContext();
From a design point of view: yes, I'd say this violates the single responsibility principle. You ...
5
Looks mostly ok, just some issues around coupling:
Consider one of the following
Pass MessageBroker.Publisher via the ApplicationClock constructor as an external dependency (preferably an interface). This will make the current implicit dependency explicit and visible which should yield in better maintenance in the future and easier unit testing.
Pass the ...
5
What is the purpose of the IMapper interface and Mapper class? It looks to me that they are just wrapping the IMappingEngine interface and MappingEngine class. While this is a good method when you have a third party class that doesn't have an interface, I think it is overkill here. Why don't you just use the IMappingEngine where you need that ...
Only top voted, non community-wiki answers of a minimum length are eligible
