Trying to clearly seperate my logic in my program

My problem is I need to know

• is my code in my CRUD functions structured well and effectively? I plan to implement CRUD for Locations entity too. hopefully without redundant code.

• you see how one CRUD function in the program class calls the other CRUD function in the DataAccess Class? I"m trying to set it up so that i can control which entity gets edited or read in the control class, and access it in the dataAccess class. Is this bad logic?

Info on classes

• Program - I'm trying to put all controlling logic here

• DataAccess - I'm trying to put my Data-accessing logic here, away from everything else.

• UserInterface - I'm trying to put my Interface logic here, away from everything else. Such as the way information gets displayed to the user, the way info gets received from user.

• InputValidation - I want to put all logic that validates user input here, ranging from Type validation to input range validation.

• ServiceManager - class that has a method that checks to see if SQL server is running, before doing LINQ calls to DataBase.

• DbResult - a repository wrapper class containing a string message, and a boolean. Used to report failure/success results with a message like "success!" or whatever exception.ToString() prints. I am thinking this class should be removed?

• Man - auto-generated class by entity framework.

• Location - auto-generated class by entity framework. I plan to implement this class in my CRUD functions as soon as I can get working clean logic for the CRUD functions that handle the Man class. And hopefully Man & Location can share the same CRUD functions.

Notes

• I'd like to enable CRUD functions to handle more than one entity class, without copying and pasting 4 CRUD functions for each new entity.

• I'm considering removing DbResult, but not sure how to without keeping Data-Accessing logic separate from User Interface logic. My thought was to pass whatever message needed to be shown to the user, to the Interface class

Since it's to show off to an employer I'll look at it in that context.

One style issue I have is that you are mixing naming conventions. You are using both hungarian notation and CamelCase. I would suggest to get rid of the hungarian notation (sParam, bIsTrue, ...).

Regarding the overall structure of your code:

-
The Program-class:

• it has too much logic (too many responsibilities). I would refactor the logic out of there into a mediator-kind of class. The Program-class should only start something.

• IMHO: the interpretation of the user input (choice 1, 2, etc) should be done by the UI class (actually if you go MVC the controller, but it looks like overkill). The UI class should expose events or delegates where the program (or better a mediator) can plugin the necessary actions.

-
The DataAccess-class:

• The DataAccess class has the role of a repository. Why not call it that. It'll show you know the repository pattern. However on second thought it looks like it has the role of a helper class?

• Use IEnumerable and yield return instead of a string array in the read method

• No disposing of the TestDatabaseEntities? It is missing from the CRUD methods on your program class.

Usually the TestDatabaseEntities has a lifetime equal to the repository, a repository encapsulates the `TestDatabaseEntities and exposes the disposing by implementing IDisposable.

-
Also in your create method why do this:

_MyDBEntities.Men.Add(new Man { ManID = M.ManID, Name = M.Name });

and not directly:

_MyDBEntities.Men.Add(M);

I hope that this is in the direction of the kind of advice you were hoping for?

I'll see if I can whip up an example later that explains better what I am trying to tell.

StackExchange Code Review Q#18858, answer score: 2