We're refactoring existing code and using sonar to analyze it.
Files feature a considerable number of get and set operators (200 to 300), increasing the file's cyclomatic complexity to 300.
This file has to be refactored to bring the threshold closer to 100.
I tried the following method:
- Partial classes do not work since the complexity is determined by the file.
- Lifting and shifting properties to many static files isn't going to work because static files can't have props?
- Should we think about any design patterns for extracting the prop and initializing it in the constructor?
What is the most efficient way to lower the Cyclomatic Complexity of such files?
Sample Code:
The class Constructor with all the properties defined in the class
public BEVehicle()
{
this.VehicleID = default(Guid);
this.VehicleZip = default(string);
this.VehicleType = default(string);
this.Year = default(string);
this.Make = default(string);
this.Make_Code = default(string);
this.Model = default(string);
this.Model_Code = default(string);
this.Trim_Code = default(string);
this.VehicleValue = default(string);
this.GrossVehicleWeight = default(string);
this.VehicleOwnership = default(string);
this.DistanceRadius = default(string);
this.AntiLockBrakes = default(string);
this.PassiveRestraintSystem = default(string);
this.AirBags = default(string);
//// RentedVehicles = default(string);
//// NonOwned = default(string);
//// ForHire = default(string);
//// VehicleFilings = default(string);
this.Comprehensive = default(string);
this.Collision = default(string);
this.Classification = default(string);
this.ClassificationDetail = new BEClassificationDetail();
this.CAClassificationDetails = new BECAClassificationDetail();
this.TrailerToVehicle = default(string);
this.State = default(string);
this.StateName = default(string);
this.City = default(string);
this.ClassificationCompany = default(string);
this.CurrentLimits = default(string);
this.VehicleUse = default(string);
this.BodyType = default(string);
this.PersonalUse = default(string);
this.VehicleWeight = default(string);
this.WorkerCompensationInsurance = default(string);
this.VehicleOperatedBy = default(string);
this.Apartment = default(string);
this.UnitType = default(string);
this.StreetAddress = default(string);
this.ISOVehicleCostNew = default(string);
this.IsVehicleCarriesGoods = default(string);
this.IsVehicleCommuteForWorkorShcool = default(string);
this.IsVehicleCommuteForWorkorSchool = default(string);
this.IsVehicleForTowing = default(string);
this.OtherTruckTypeDescription = default(string);
this.TruckType = default(string);
this.GoodsPickUpLocation = default(string);
this.IsVehicleCustomized = default(string);
this.ValueOfCustomization = default(string);
this.Zone1 = default(string);
this.Zone2 = default(string);
this.SecondaryCode = default(string);
this.SelectedVehicleType = default(string);
this.ISOVehicleType = default(string);
this.DistanceRadiusEntered = default(string);
this.ISOClassificationDetails = new BEISOClassificationDetail();
this.IsDumpTrailer = default(string);
this.PentonVehicleType = default(string);
this.SICCode = default(string);
this.SICCodeTitle = default(string);
this.NicoKickOutReason = default(string);
this.MinimumGVW = default(string);
this.MinimumVehicleValue = default(string);
this.VIN = default(string);
this.AuxRunning = default(string);
this.AlarmDiscount = default(string);
this.AntitheftDeviceinstalled = default(string);
this.BusinessUse = default(string);
this.NJInEligible = default(string);
this.NJInEligibleReason = default(string);
this.RegisteredOwner = default(string);
this.RegistrationType = default(string);
this.PENTONTAB = default(string);
this.PENTONVEH_TYPE = default(string);
this.HazardousWaste = default(string);
this.TypeOfDelivery = default(string);
this.NumberOfJobsites = default(string);
this.TowHaulAutos = default(string);
this.StrVehicleNotListed = default(string);
this.RegisteredState = default(string);
this.RegisterInThirtyDays = default(string);
this.DaysPerWeekRS = default(string);
this.TripsPerDayRS = default(string);
this.TransportRideshare = default(bool);
this.TransportOutsideRideshare = default(bool);
this.PersonalUseRideshare = default(bool);
this.Commuting = default(bool);
this.RSSeatCapacity = default(string);
this.CommuteMilesRS = default(string);
this.MilesPerTripRS = default(string);
this.ActualVehicle = default(string);
this.SelectedAnnualMileage = default(string);
this.CalculatedAnnualMileage = default(string);
this.CalculatedRSMileage = default(string);
this.CalculatedPersonalMileage = default(string);
this.CalculatedCommuteMileage = default(string);
this.OtherFirstname = default(string);
this.OtherLastname = default(string);
this.OtherSuffix = default(string);
this.UninsuredPropertyDamage = default(string);
this.County = default(string);
this.VehAddressVerifiedInd = default(string);
this.VehInEligible = default(string);
this.VehInEligibleReason = default(string);
this.PCAVehicleCostNew = default(string);
this.LienholderForCustomEquipment = default(string);
this.NonDeductibleGlassCoverage = default(string);
this.LocationClientId = default(int);
this.SourceLocationId = default(string);
}
Individual properties with get and set properties
/// Gets or sets VehicleOwnership.
/// </summary>
/// <value>
/// The vehicle ownership.
/// </value>
[XmlElement(ElementName = "VehicleOwnership", Order = 11)]
public string VehicleOwnership { get; set; }
/// <summary>
/// Gets or sets DistanceRadius.
/// </summary>
/// <value>
/// The distance radius.
/// </value>
[XmlElement(ElementName = "DistanceRadius", Order = 12)]
public string DistanceRadius { get; set; }
/// <summary>
/// Gets or sets AntiLockBrakes.
/// </summary>
/// <value>
/// The anti lock brakes.
/// </value>
[XmlElement(ElementName = "AntiLockBrakes", Order = 13)]
public string AntiLockBrakes { get; set; }
/// <summary>
/// Gets or sets PassiveRestraintSystem.
/// </summary>
/// <value>
/// The passive restraint system.
/// </value>
[XmlElement(ElementName = "PassiveRestraintSystem", Order = 14)]
public string PassiveRestraintSystem { get; set; }
/// <summary>
/// Gets or sets AirBags.
/// </summary>
/// <value>
/// The air bags.
/// </value>
[XmlElement(ElementName = "AirBags", Order = 15)]
public string AirBags { get; set; }
CodePudding user response：
I think the most efficient way is rewrite that huge class in sub classes with part of the properties then your class BEVehicle have properties with sub classe i.e:
public class BEVehicleModel
{
public string VehicleZip { get; set;}
public string VehicleType { get; set;}
public string Year { get; set;}
public string Make { get; set;}
public string Make_Code { get; set;}
public string Model { get; set;}
public string Model_Code { get; set;}
public string Trim_Code { get; set;}
}
then in BEVehicle
public class BEVehicle
{
public Guid VehicleId { get; set; }
public BEVehicleModel VehicleModel { get; set; }
public BEVehicle()
{
//your initialization
}
}
CodePudding user response：
Why do you initialize for example strings with default(string) which is null and every string property is already null if you do not touch it. Your code looks more like created by some codegenerator. If it's handwritten I would recommend to remove everthing that does nothing like setting a bool to false or string to null or int to 0. If the code does not change anything in your class it just distracting you from seeing the relevant parts. Your constructor could be shortend to the following without changing the behaviour of your class.
public BEVehicle()
{
this.ClassificationDetail = new BEClassificationDetail();
this.CAClassificationDetails = new BECAClassificationDetail();
this.ISOClassificationDetails = new BEISOClassificationDetail();
}
and you should delete all the comments if the do not add any information that helps understanding the code.
Stating the obvious is distracting instead of helping comments like this should be avoided
// asign value 42 to int
int foo = 42;
and your xml decorations could be made more refactoring save
[XmlElement (ElementName = nameof(VehicleOwnership), Order = 11)]
public string VehicleOwnership { get; set; }
or if the Order is not relevant for you you could change the code to (but strictly speaking that would be more than refactoring because this could change the order in which the elements get serialized)
[XmlElement]
public string VehicleOwnership { get; set; }
So the result is a extremly simple class. If you have rules that prohibit classes like that you should consider changing your rules or at least exclude simple DTOs from complexity checks