C# – Using Partial Classes to Hide Controller Complexity

c

I'm a quite a junior programmer, but happy to learn! 🙂 I put together a WebApi based application and I sent it to be reviewed by my colleague. I got it back with the opinion the way I use partial classes hide controller complexity and size. I'm really happy to hear his opinion (he has been a developer for more that 15 years) because I respect him. But I like to cross-check opinions, viewpoints to see as many aspect of a case as possible.

You can find the examples and screenshots below about the code.

The reasons why I organize my stuff are the next:

  • easy to find the controller method in solution explorer
  • one file ( one partial class ) contains only a single method and its internal utility methods (if there is any). If a utility method is used by multiple controller methods, then I organize them in a common place
  • a controller is responsible for a single endpoint (in the example you can see the Environment controller which is responsible for environment sub-domain)
  • a method is never longer than a common screen size for two reasons: a, I hate scroll up and down, b, controller business logic is extracted to a separated library called Application, the controller responsible only for call the application method and return the result.

My colleague mentioned that, partial classes are awesome when a class implements multiple interfaces and you can separate the interface implementations into partial classes.

My questions:

  • it is enough information and code sample I provided?
  • it is a good approach I have?
  • if not then where are the weak points of it and why?
  • what is the common pattern in these area, however, code review is rather a common set of opinions and culture of a team/company
  • where is the healthy trade-off between readability/manageability (according to the code review result I pushed this part a little bit to far) and hiding complexity, violating responsibility rules

EnvironmentController.cs file: it is responsible (it contains only) for dependency injection related setup (constructor injection), listing fields and having the RoutePath value.

[RoutePrefix("Environment/Environments")]
    public partial class EnvironmentController : ApiController
    {
        private IEnvironmentApplication environmentApplication;

        public EnvironmentController(IEnvironmentApplication environmentApplication)
        {
            this.environmentApplication = environmentApplication;
        }
    }

EnvironmentController.AddNewEnvironment.cs: responsible only for AddNewEnvironment method and possible internal utility methods, if there is any

public partial class EnvironmentController
    {
        [HttpPost]
        [Route("AddNewEnvironment")]
        public HttpResponseMessage AddNewEnvironment([FromBody] string environmentViewModelString)
        {
            try
            {
                EnvironmentViewModel model = JsonConvert.DeserializeObject<EnvironmentViewModel>(
                    environmentViewModelString,
                    new JsonSerializerSettings { NullValueHandling = NullValueHandling.Ignore });

                EnvironmentContract result = this.environmentApplication.AddNewEnvironment(model);
                return this.Request.CreateResponse(HttpStatusCode.Accepted, result);
            }
            catch (Exception e)
            {
                return this.Request.CreateErrorResponse(HttpStatusCode.BadRequest, e);
            }
        }
    }

How it looks like in solution explorer.

enter image description here

Best Answer

Your approach is not common practice and may raise a few issues, since Visual Studio is optimized to work a different way.

easy to find the controller method in solution explorer one file ( one partial class ) contains only a single method and its internal utility methods (if there is any).

Visual Studio provided dropdowns in the top of the code pane that allow you to quickly find a method. Also, VS supports regions that can be named, nested, hidden, and shown as needed.

If a utility method is used by multiple controller methods, then I organize them in a common place

What about a utility method that was previously defined in one of your method files, but later you discover you need to call it from a new method? You'd have to move it. Not only is that a PITA, but it will break history in your code repository.

a method is never longer than a common screen size for two reasons

No reasons necessary; short methods are a good practice in general. One way to keep methods short is to break them into smaller methods. But-- using your approach, there is actually a disincentive to break up longer methods, because the developer will have to add a new file each time. God help a developer who is restructuring code and wants to to be able to add/remove/restructure a whole lot of methods at once.

The only advantage I can think of is this: in the "one file" approach, if a developer decides to move a method around (e.g. move it from the bottom to the top) it can cause code merges to get confused. If there is only one method per file, this is a non-issue. But-- on the other hand, if a method name changes, under your system the file name would change as well, and merges might get skipped entirely.

Related Topic