Hot answers tagged

15 votes
Accepted

Throwing exceptions when validation fails

You're throwing System.Exception. Don't do that. If you're going to have to throw an exception for a validation exception, throw a custom ...
user avatar
13 votes
Accepted

Proper handling of exceptions in MVP

Short Answer: Having a call to MessageBox.Show() in any DAL code is A Bad Thing (TM). Longer Answer: In an ideal world, your DAL would not know anything about ...
user avatar
  • 3,589
13 votes

Calculate balance wages

Formatting formatting inside your foreach should be indented This: ...
user avatar
  • 28.5k
10 votes

Calculate balance wages

It seems to me that you're hard-coding far too much here. IMO, this type of task should be heavily database driven. For example, each item of earning and each deduction should have an amount, and a ...
user avatar
  • 33.2k
9 votes

Throwing exceptions when validation fails

...
user avatar
  • 30.7k
8 votes
Accepted

Exploring the code of the Code Explorer

(Not a full review, just focusing on one method.) I think private string GetImageKeyForDeclaration(Declaration declaration) contains a lot of repeated code that ...
user avatar
  • 10.6k
8 votes
Accepted

Chutes & Ladders Board Generator (June 2016 Community Challenge)

class Generator ...
user avatar
  • 49.9k
7 votes
Accepted

Populating a Data Grid View in MVP

For the simplicity I'm not using a "model class" for this [...] Then it's not Model-View-Presenter you have here. ...
user avatar
7 votes

Proper handling of exceptions in MVP

You don't need these lines of code at all if (sqlConnection.State == System.Data.ConnectionState.Open) sqlConnection.Close(); return false; The point of using ...
user avatar
  • 28.5k
7 votes
Accepted

Down the rabbit hole with MVP

There's usually very little I can say about your code, but maybe if I just start going down line by line I'll find something. I'm hoping someone else who knows more about mvp and dependency-injection ...
user avatar
  • 30.7k
6 votes
Accepted

Calculate balance wages

It looks like you have a pair of lists which you iterate through based on the elements contained in the earningList. This is potentially dangerous without ...
user avatar
  • 3,096
6 votes

DAL Returns a collection of business objects in MVP

You don't want to pull the entire Employee table unless necessary. ...
user avatar
  • 3,096
6 votes
Accepted

Follow-Up Post Tictactoe now with AI

Fields I see a property Cells but it is never used either in Grid or any other class (as far as I can tell). Order your class ...
user avatar
6 votes

Input validation for text boxes in a Form

...
user avatar
6 votes
Accepted

Honey I shrunk the view

I really like this method of declaring a bitwise enum. ...
user avatar
  • 30.7k
6 votes

Throwing exceptions when validation fails

You shouldn't throw Exception, create a custom Exception as proposed @Mat's Mug and throw that one instead, otherwise you might trap an exception you didn't want to trap (ex : ...
user avatar
  • 12.2k
6 votes

Implementing both poor-man's MVP and MVPVM framework

public abstract class Presenter<V, VM, M> where V : IView<VM, M> where VM : ViewModel<M> where M : IModel Convention for single-...
user avatar
6 votes

Does this method contain multiple implementations?

Seemed fine at first, then I took another look... The validation function displays the error message? I'm sorry, but that's indeed a violation of SRP. You'd be better off returning a ...
user avatar
  • 22.2k
6 votes
Accepted

MVP Passive-View

Your view is calling methods on your presenter. The view shouldn’t know the presenter exists. Instead of calling the presenter directly, the view should raise events that the presenter reacts to. The ...
user avatar
  • 30.7k
5 votes

Employee wage/salary calculation MVP solution

This method smells: ...
user avatar
5 votes

Down the rabbit hole with MVP

There's an itchy spot with extensibility: because an implementing class must implement all members of the IPresenter interface, adding new commands is quickly ...
user avatar
5 votes

Exploring the code of the Code Explorer

I'm not going to make a full review either - I'll leave that to other eyes. One thing I just have to mention is this part of the async void RefreshTreeView method: ...
user avatar
5 votes
Accepted

Simple dictionary storing/viewing application

It's hard to review good code, and this is relatively good code. Things are generally well named and no methods are overly long. You've done well, but there is room for improvement. Let's start at ...
user avatar
  • 30.7k
5 votes

Chutes & Ladders Board Generator (June 2016 Community Challenge)

Your algorithm for getting a collection of random deltas could be improved. It checks for collisions each number it adds, which is O(n) each time, and it keeps creating a new random number until ...
user avatar
  • 1,601
5 votes
Accepted

TicTacToe in MVP Winforms

To begin with... ...
user avatar
  • 43.8k
4 votes
Accepted

DAL Returns a collection of business objects in MVP

Your Question System.Data.DataTable lives under the System.Data namespace; one good way to start mixing concerns in your BLL is ...
user avatar
4 votes
Accepted

Using Models and Entities in MVP pattern

It's not clear exactly what you're referring to as Model classes, I think you're over-thinking it. I'd go with the else snippet, the POCO (Plain Old CLR Object) that does nothing but exposing auto-...
user avatar
4 votes
Accepted

Using built-in type Decimal or Entity type Advance, to hold AdvanceAmount

This question is very, very similar to another question you recently asked, so this answer will basically (hopefully!) clarify my other answer. KISS: Keep It Simple, Stupid. These classes represent ...
user avatar
4 votes

Server side validations and unit testing in MVP

I think you have a typo in the last outside if statement, I assume you meant two different variables when you wrote this: ...
user avatar
  • 28.5k

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