47181

Good Coding Practices [closed]

Question:

I am working on my 2nd project with Entity Framework and I would like to read some opinions about if the code below its good coding practice of not.

My proposed architecture is this:

<img alt="enter image description here" class="b-lazy" data-src="https://i.stack.imgur.com/LP6bl.png" data-original="https://i.stack.imgur.com/LP6bl.png" src="https://etrip.eimg.top/images/2019/05/07/timg.gif" />

So, the website calls the business logic, business rules are evaluated there. THen it calls the DALFacade, its just that its invisible for the upper layers whether if we are accessing db2 or sql server.

The EF DAL, its the one in charge of talking with database, CODE FIRST Approach. The entities class library is the project with the poco classes. Utilities its self explanatory, I might put there code like reading properties from active directory.

SO here is my code, I simplified it as much as I could

OnePage:

protected void BtnSubmitRequestClick(object sender, EventArgs e) { var ecoBonusWorkflow = new EcoBonusWorkflow { IsOnHold = true }; BusinessLogic.EcoBonusRequestSave(ecoBonusWorkflow); }

BUsiness Logic:

public static class BusinessLogic { public static void EcoBonusRequestSave(EcoBonusWorkflow ecoBonusWorkflow) { if (true) { DalFacade.EcoBonusRequestSave(ecoBonusWorkflow); } } }

DALFacade:

public static class DalFacade { private static readonly UnitOfWork UnitOfWork = new UnitOfWork(); public static void EcoBonusRequestSave(EcoBonusWorkflow ecoBonusWorkFlow) { UnitOfWork.EcoBonusWorkflowRepository.InsertEcoBonusWorkflow(ecoBonusWorkFlow); } }

Then in the EF Dal class library I use the traditionary EF 4.1 Code First Approach. I used also Repository Pattern and Unit of Work.

Any observation is welcomed

Answer1:

The inclusion of 'utilities' seems very strange to me. Everything else is decently well named with a pretty specific setup of what exactly it should do, then you have this vaguely named block of 'utilities'.

This isn't any particular coding standard, but I would avoid incorporating 'utilities' into your top level architecture as much as you possibly can. Every project has some folder like that named 'helpers' or 'utilities' and it ends up being a dump for a large amount of miscellaneous code that doesn't seem like it immediately fits anywhere else. This is exactly the way technical debt and spaghetti code begins.

It will probably end up happening no matter what you present unless you are the only developer on the project, but nevertheless I'd avoid legitimizing it.

Also, the inclusion of static classes in your quick-and-dirty alarms me. Aside from the fact that static code hurts testability, the Entity framework is <em>not</em> thread-safe. It is up to you to use it in a thread-safe way, so if you have any multithreading or co-processing going on at all your DALFacade is going to randomly throw a ton of errors and be a gigantic headache.

I also don't see any allowance for dependency injection in your design, which again goes to how testable the code is.

EDIT: OK, these were not concessions for Stack, so it's time to introduce the idea of <a href="http://msdn.microsoft.com/en-us/magazine/cc163739.aspx" rel="nofollow" title="depenency injection">dependency injection</a>.

Basically, any time you make something directly depend on something else, such as when you invoke a static class, you are tying these two things together. One part is no longer replaceable without also modifying or replacing the other part. This is A Bad Thing, because it makes refactoring and changes a very large problem. What you want to do instead is to 'loosely couple' these dependencies so you can change parts out without breaking everything. In C#/.NET, you mostly do this with interfaces.

So instead of having "DALFacade" and passing it around directly, you would instead have something to the effect of:

interface IDalFacade { int DoAThing(); bool DoAnotherThing(); } class DalFacade : IDalFacade { public int DoAThing() { return 0; } public bool DoAnotherThing() { return false; } }

You may then use it in your business logic like so:

class BusinessLogic : IBusinessLogic { public IDalFacade DalFacade { get; private set; } public BusinessLogic() : this(new DalFacade()) {} public BusinessLogic(IDalFacade dalFacade) { this.DalFacade = dalFacade; } public void LogicTheThings() { this.DalFacade.DoAThing(); } }

It now no longer matters if DalFacade gets completely trashed or if all of a sudden you need two of them for different things. Or even, truly, if a DalFacade is even passed in. As long as it fulfills the IDalFacade contract, it's perfectly fine. You can still call it with no parameters (it just assumes a specific implementation), but even this is not ideal. Ideally you would want to use a dependency injection library, such as <a href="http://www.ninject.org" rel="nofollow">Ninject</a> or <a href="http://structuremap.net" rel="nofollow">StructureMap</a>, which would allow you to make any specific changes even more easily.

This is also useful when attempting to unit test your code, since, as I said, you don't even need an actual DalFacade to do things. You can simply mock out something that fills the IDalFacade contract (using something like <a href="http://ayende.com/wiki/Rhino+Mocks.ashx?AspxAutoDetectCookieSupport=1" rel="nofollow">RhinoMocks</a>), and you can then test all your code totally separately from anything else in your project.

Recommend

  • Instanciate service on startup in Angular2
  • Using constants or global variables in 3 tier console application
  • Share two different things in Windows Phone 8.1 C#
  • Google Maps getMap returns null
  • How can i match particular format in input using java.util.regex in java?
  • Hibernate Joda DateTime Sorting
  • Run EF6 Query in separate Thread on WinForm Button Click Event
  • DependencyObject.AssociatedObject is always null
  • WPF: When is a TabItem actually loaded, rendered and completly ready?
  • How does inheritance and polymorphism work in this situation?
  • How to change the default configuration files used in bootstrapping of reactJs through npm
  • Want to save selected (i.e., more than 1) enums as string with NHibernate
  • Exception HRESULT: 0x800455BC in speech recognition in Windows phone 8
  • Convert RSA pem key String to der byte[]
  • Accessing the variables from a PHP Anonymous Function
  • Magento: where is the trigger of the custom cache?
  • How can I prevent the need to copy strings passed to a avr-gcc C++ constructor?
  • Android: How to correctly use NotifyDataSetChanged with SimpleExpandableListAdapter?
  • Select value from xtype selection type checkbox CQ5
  • Migration to HRD - How to convert string-encoded keys to new application
  • Correct implementation of List Iterator methods
  • drawing random circles, storing their coorindates in an array
  • Primefaces ManyCheckbox inside ui:repeat calls setter method only for last loop
  • Does Apple allow the usage of sysctl.h within iOS applications?
  • Regarding starting the threads on a condition
  • Implementing “partial void” in VB
  • How do I signal completion of my dataflow?
  • c# open webrowser in many tab
  • Instantiate interface in JAVA?
  • how to avoid repetitive constructor in children
  • How to delay loading a property with linq to sql external mapping?
  • Disabling Alt-F4 on a Win Forms NotifyIcon
  • Can I display google adwords (AdView) in javafx on android
  • Uncaught Error: Could not find module `ember-load-initializers`
  • Opengl-es onTouchEvents problem or a draw problem? [closed]
  • output of program is not same as passed argument
  • Javascript + PHP Encryption with pidCrypt
  • Getting Messege Twice Using IMvxMessenger
  • How can i traverse a binary tree from right to left in java?
  • How can I use threading to 'tick' a timer to be accessed by other threads?