We’re rewarding the question askers & reputations are being recalculated! Read more.
4

The repository does not need to take responsibility for the errors - remove them from here. The flight repository should add CRUD operations to Flight Dto and not return a result - remove the result. Validation should be done before calling the methods. You are not an automapper, use a tool. You do not need to invent a bicycle every time you need a new ...


3

Review Your design uses a dedicated repository and service for each entity. This might look like a great idea at first, but you'll soon realise the drawbacks of this design. Having a service as a glorified wrapper of a repository doesn't add much value to the service. I would question whether a service should yield IQueryable. I find a service to be in ...


3

So now basically rewriting the whole answer. As OP asks: Basically I'm looking for a way to [...] avoid making a lot of if statements and parameter combination manually This answer provides an alternate solution to tackle exactly that. Precisely, it is easier to maintain and can be adapted to other models as well. In the whole code I only found a ...


3

Some quick remarks: Why isn't PaidDate a DateTime? What is a "Shopping"? (I realize that often naming things is hard, but be careful when inventing new English words that do not adequately describe their contents, because the next person to maintain this code will find it harder to do so when they have to first decipher various terms.) Use descriptive names:...


3

It might be boring to hear it again, but I have to ask :) Why do you think your application need performance optimisations? Is your bottle neck in these parts of code which you provided? Maybe problem is with database/external service/network? Have you measured processing and communication time? Stopwatch Do you have some kind of monitoring of application, ...


3

SOLVED this is the way I ended up doing it, don't know if it's the best way, but it works I took this idea from an answer that was posted here but now it's not anymore, and Honestly I don't remember who suggested it, I took the foreach loop and adapted to my case, for my point of view the foreach loop is the fasted way, and also I can turn it into a ...


2

Unneeded complexity There is unnecessary complexity because of two reasons: the unit of work wants to be a repository locator and because the repositories are generic on the wrong side. Remove the repositories references from the UoW and you will be fine. Let's see now a way to have a better generic way for your CRUD. public interface IRepositoryBase { ...


2

It is a best-practice to have a clear separation between UI layer (the forms) and data access, so all your data access logic (opening the connection, issuing quries, closing it etc.) should be handled in a separate class (service) that can be reused by other classes / forms. This will help you have a single place to handle specific things like logging, ...


2

You don't need the grouping at the Client level. No grouping at all, actually. Don't worry, this is a common mistake when people want grouping/aggregation in child collections. Per client, the Tickets can be counted: return clients .Where(cli => cli.IsActive) .Select(cli => new DashboardItemViewModel { Id = cli.Id, Name = ...


2

Yes. You are close, man! You have to make relashionships between pairs of controller+action to some user actions. For example: Security Staff can only view Personal Info: - View Personal Then your HHRR Manager can: - View Personal - Add Personal - Edit Personal And finally HHRR Director he can do more actions like: View Personal Add Personal Edit ...


2

It is safe to use the context the way you are using it. However, I don't think you will be able to write unit test. This isn't bad in itself. It does become an issue when your business logic is middle tier. Also why aren't you all using the rest standard instead of your perform method? Rest leaverages http to make your api more clear and interoperable.


2

DRY Principle You should write DRY (don't repeat yourself) code. Each of your if-blocks contains the following pattern: using (ELECTROPEntities db = new ELECTROPEntities()) { try { var x = getData(db) .Where(predicate).ToList(); return PartialView("multi", x); } catch (Exception ex) { return Json(...


1

First off, the Include is ignored because you project the result to a new result (Select(x => new{ ... }). So you may as well remove it. Then, there is some room for improvement by eliminating the repetitive parts. It's much easier to do this in query syntax: var customers = from cst in _customerRepository.All() // Assuming this method exists let ...


1

It's hard to answer your question with a definitive answer as so much information is missing and a lot, depends also on the other UI screens data that you need to fill or that other may have opened. You’re going to get issues when performing aggregates on the same table that you provide row data on as you might end-up in a deadlock, even with yourself. I'm ...


1

So for anyone that might find it interesting: I found how to make it more efficient. I used as data source for my data grid this: dgActions.DataSource = db.Actions.Where( x => x.Client_ID.Equals(Main.client.Client_ID)).ToList<Action>(); But in my opinion that is not so efficient. So I thought to use the navigation property of the class ...


1

I've thought about a factory method, but that won't work because it can't be translated to SQL by the expression interpreter. Why don't you call (as suggested by t3chb0t) AsEnumerable() right before your factory method? You would perform the factory method in memory. var project = entities.Tickets .Where(p => p.ID == id) .AsEnumerable(...


1

You should not instanciate a new DbContext each time your code go throw your ActionFilter. What you should do is to use dependency injection and to define an execution scope. Because you are using .net Framework and not .net core, I advise you to look into DI providers such as Autofac or Ninject. I advise you to look into why to use DI and think about ...


1

Your code does not work the way you think it does. .GroupBy(cli => cli, cli => cli.Tickets) This is not grouping anything because you are using the entire object as a key and since you are not providing any comparer for them each group will have exactly one item, itself. .Select(grp => Then from each one-itemed-group you select its single item ...


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