I got the following code analysis warning message and I am having no clue to get rid that issue. Please advise.
CA1506 Avoid excessive class coupling
'RetailerController.RetailerFormSummary(string)' is coupled with 33 different types from 13 different namespaces. Rewrite or refactor the method to decrease its class coupling, or consider moving the method to one of the other types it is tightly coupled with. A class coupling above 40 indicates poor maintainability, a class coupling between 40 and 30 indicates moderate maintainability, and a class coupling below 30 indicates good maintainability. OnePlaceWebApp.Web RetailerController.cs 564
The method RetailerFormSummary(string) looks like:
public ViewResult RetailerFormSummary(string retailerId)
{
AlertService alertService = new AlertService();
PageInfo winnersPageInfo = new PageInfo();
RetailerFormModelSummary model = new RetailerFormModelSummary();
winnersPageInfo.ItemsPerPage = _configurationUtility.SummaryWinnersCount;
winnersPageInfo.PageIndex = 0;
model.CurrencyFormat = _configurationUtility.CurrencyFormat;
model.DateFormat = _configurationUtility.DateFormat;
model.DatePickerFormat = _configurationUtility.DatePickerFormat;
model.Reps = _userService.GetReps(_userService.GetCurrentWebAppUserId());
model.UsesDistricts = _configurationService.GetUsesDistricts();
RetailerDto retailer = _retailerService.SelectRetailer(retailerId);
if (retailer != null)
{
RetailerJurisdictionDto retailerJurisdiction = _retailerSummaryService.SelectRetailerJurisdiction(retailer.Id);
JurisdictionDto territory = null;
JurisdictionDto district = null;
JurisdictionDto region = null;
DateTime? lastVisit = _retailerSummaryService.SelectLastVisitUtcDate(retailer.Id);
if (retailerJurisdiction != null)
{
territory = _jurisdictionService.SelectJurisdictionUsingIdAndType(retailerJurisdiction.JurisdictionId, JurisdictionTypes.Territory);
district = _jurisdictionService.SelectJurisdictionUsingIdAndType(retailerJurisdiction.JurisdictionId, JurisdictionTypes.District);
region = _jurisdictionService.SelectJurisdictionUsingIdAndType(retailerJurisdiction.JurisdictionId, JurisdictionTypes.Region);
}
model.Address1 = retailer.Street1;
if (retailer.Street2 != null)
{
model.Address2 = retailer.Street2;
}
if (retailer.CorporateAccount != null)
{
model.CorporateAccount = FormatService.GetTwoPartName(retailer.CorporateAccount.Number, retailer.CorporateAccount.Name, " - ");
}
model.CorporateAccountName = _termsService.SelectSingularTermValue(TermTypes.CorporateAccount);
model.City = retailer.City;
if (district != null)
{
model.District = FormatService.GetTwoPartName(district.Number, district.Name, " - ");
}
if (lastVisit.HasValue)
{
model.LastVisit = lastVisit.Value.DayOfWeek.ToString() + " " + lastVisit.Value.ToLocalTime().ToString(model.DateFormat, CultureInfo.CurrentCulture);
}
model.Note = _retailerSummaryService.SelectRetailerComment(retailer.Id);
model.Phone = FormatService.FormatPhoneNumber(retailer.Phone);
if (region != null)
{
model.Region = FormatService.GetTwoPartName(region.Number, region.Name, " - ");
}
model.RegionTerm = _termsService.SelectSingularTermValue(TermTypes.Region);
model.RetailerId = retailer.Id.ToString(CultureInfo.CurrentCulture);
model.RetailerName = retailer.BusinessName;
ViewBag.Title = retailer.BusinessName;
model.RetailerNumber = retailer.DisplayNumber;
model.RetailerAlerts = alertService.GetRetailerAlertsStringArray(retailer.RetailerAlerts);
model.Route = _routeManagerService.GetRouteStringForRetailer(retailerId);
model.Tasks = _retailerSummaryService.SelectRetailerTasks(retailer.Id);
if (territory != null)
{
model.Territory = FormatService.GetTwoPartName(territory.Number, territory.Name, " - ");
}
model.TerritoryTerm = _termsService.SelectSingularTermValue(TermTypes.Territory);
model.WeeklySales = _retailerSummaryService.SelectWeeklySales(retailerId);
model.Winners = _retailerWinnersService.SelectWinners(retailerId, WinnersSortTypes.Date, winnersPageInfo);
model.ZipCode = retailer.ZipCode;
}
return View("RetailerFormSummary/RetailerFormSummary", model);
}
This warning is telling you that instantiating dependent concrete objects within your method can lead to maintainability issues.
Here are some examples:
AlertService alertService = new AlertService();
PageInfo winnersPageInfo = new PageInfo();
RetailerFormModelSummary model = new RetailerFormModelSummary();
RetailerJurisdictionDto retailerJurisdiction = _retailerSummaryService.SelectRetailerJurisdiction(retailer.Id);
JurisdictionDto territory = null;
JurisdictionDto district = null;
JurisdictionDto region = null;
This method has too many responsibilities and owns too many dependencies. You should definitely look up the Dependency Inversion Principle and perhaps an Inversion of Control Framework to help with your dependency injection. This will help reduce the coupling in your classes/methods.
The mantra is "depend upon abstractions".
Also, the Single Reponsibility Principle applies here as well.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With