3
\$\begingroup\$

I have the following two methods that write to file various data line by line. Both methods differ from each other only by few lines what would be the best way to refactor them that when in future I want to add another method (for example, CreateXYZLegacyInFile) it would be least time consuming.

public string CreateCVTLegacyInFile()
{
 try
 {
 string dllLocation = string.Format("{0}\\{1}\\cvt\\", dllWorkingDirectory, FactoryCode);
 string inFile = string.Empty;
 inFile += AddLineWithMarks(dllLocation);
 inFile += AddLineWithMarks(mainData.CalculationReference);
 inFile += AddLineWithMarks(DateTime.Now.ToString("MM/dd/yy").Replace('-', '/'));
 inFile += AddLineWithMarks(mainData.Designer);
 inFile += AddLineWithMarks(mainData.TransformerType);
 inFile += AddLine(mainData.TransformerSize);
 inFile += AddLineWithMarks(mainData.TransformerModel);
 inFile += AddLineWithMarks(mainData.MainStandard);
 inFile += AddLineWithMarks(mainData.SecondaryStandard);
 inFile += AddLine(mainData.Frequency);
 inFile += AddLine(mainData.HighestSystemVoltage);
 inFile += AddLine(mainData.PowerFrequencyInsulationLevel);
 inFile += AddLine(mainData.LightningInsulationLevel + "," + mainData.SwitchingInsulationLevel);
 inFile += AddLineWithMarks(vtData.RatedVoltage);
 inFile += AddLineWithMarks(capacitanceData.NominalIntermediateVoltage);
 inFile += AddLine(vtData.VoltageFactor);
 inFile += AddLineWithMarks(vtData.VoltageFactorTime);
 inFile += AddLine(mainData.MinimumAmbientTemperature + "," + mainData.MaximumAmbientTemperature + "," + mainData.AverageAmbientTemperature);
 inFile += AddLineWithMarks(capacitanceData.CVDType);
 inFile += AddLineWithMarks(capacitanceData.EMUType);
 inFile += AddLine(capacitanceData.EquivalentCapacitance);
 inFile += AddLine(capacitanceData.TotalNumberOfElements + "," + capacitanceData.NumberOfElementsSelected);
 inFile += AddLine(capacitanceData.CapacitanceTotal + "," + capacitanceData.CapacitanceSecond + "," +
 capacitanceData.CapacitanceFirst + "," + capacitanceData.CapacitanceElement);
 inFile += AddLine(mainData.InsulatorCreepageDistance + "," + mainData.InsulatorFlashoverDistance);
 inFile += AddLineWithMarks(mainData.InsulatorType);
 inFile += AddLineWithMarks(mainData.InsulatorColor);
 inFile += AddLineWithMarks(mainData.PrimaryMarkings);
 inFile += AddLineWithMarks(capacitanceData.IsPotentialGroundSwitch);
 inFile += AddLineWithMarks(mainData.IsAutoMode);
 inFile += AddLineWithMarks(mainData.IsSpecialDesign);
 if (vtData.IsSpecialDesign) 
 {
 string areEnabledMarkings = string.Empty;
 areEnabledMarkings += SpData.IsSelectionOfSecondaryTurns + ",";
 areEnabledMarkings += SpData.IsSelectionOfPrimaryWire + ",";
 areEnabledMarkings += SpData.IsSelectionOfSecondaryWire;
 inFile += AddLine(areEnabledMarkings);
 if (SpData.IsSelectionOfSecondaryTurns == "T") 
 {
 inFile += AddLine(SpData.SecondaryTurnFirstWinding);
 }
 if (SpData.IsSelectionOfPrimaryWire == "T") 
 {
 inFile += AddLineWithMarks(SpData.PrimaryWindingWireDimension);
 }
 if (SpData.IsSelectionOfSecondaryWire == "T") 
 {
 foreach (var item in SpData.SecondaryWindingWireDimensions)
 {
 inFile += AddLineWithMarks(item.SecondaryMainWindingWireDimension);
 inFile += AddLineWithMarks(item.SecondaryTappedWindingWireDimension);
 }
 }
 }
 inFile += AddLineWithMarks(mainData.SerialNumberPrefix.ToString());
 inFile += AddLineWithMarks(mainData.SerialNumberSuffix.ToString());
 inFile += AddLineWithMarks(string.Empty); //XLNUMSUB ??
 inFile += AddLine(windingsData.Count.ToString());
 foreach (WindingData windingData in windingsData)
 {
 if (vtData.IsExtendedNumberOfClasses)
 {
 inFile += AddLineWithMarks("X");
 inFile += AddLine(windingData.NumberOfClassRequests);
 }
 inFile += AddLineWithMarks(windingData.TapMarking);
 inFile += AddLineWithMarks(windingData.EarthMarking);
 inFile += AddLineWithMarks(windingData.NominalVoltageTapped);
 inFile += AddLineWithMarks(windingData.NominalVoltageMain);
 inFile += extractBurdenAndAccuracyClassData(windingData.MainTerminalBurdenAndAccuracy);
 inFile += extractBurdenAndAccuracyClassData(windingData.MainTerminalOtherWindingBurdenAndAccuracy);
 if (vtData.IsExtendedNumberOfClasses)
 {
 inFile += extractBurdenAndAccuracyClassData(windingData.TappedTerminalBurdenAndAccuracy);
 inFile += extractBurdenAndAccuracyClassData(windingData.TappedTerminalOtherWindingBurdenAndAccuracy);
 }
 inFile += AddLineWithMarks(windingData.Fuse);
 foreach (string terminalMarking in windingData.TerminalMarking)
 {
 inFile += AddLineWithMarks(terminalMarking);
 }
 }
 string gyrStatus = string.Empty;
 gyrStatus += " ";//ALERT TOTAL
 gyrStatus += translateGYRStatus(commonTransformerProperties.TransformerTypeAndSizeStatus);
 gyrStatus += translateGYRStatus(commonTransformerProperties.AverageAmbientTemperatureStatus);
 gyrStatus += translateGYRStatus(capacitorVoltageDivider.Insulator.Status);
 gyrStatus += " "; //Shell NO
 gyrStatus += " "; //Ipn
 gyrStatus += " "; //OMS
 gyrStatus += (intermediateVoltageTransformer.IsPotentialGround) ? "Y" : "G";
 gyrStatus += translateGYRStatus(intermediateVoltageTransformer.Status);
 gyrStatus += translateGYRStatus(capacitorVoltageDivider.CVDStatus);
 gyrStatus += translateGYRStatus(secondaryWindings.FuseStatus);
 gyrStatus += " ";// Alert STATUS
 inFile += AddLineWithMarks(gyrStatus);
 return inFile;
 }
 catch (Exception ex)
 {
 throw ex;
 }
}
public string CreateVTLegacyInFile()
{
 try
 {
 string dllLocation = string.Format("{0}\\{1}\\vt\\", dllWorkingDirectory, FactoryCode);
 string inFile = string.Empty;
 inFile += AddLineWithMarks(dllLocation);
 inFile += AddLineWithMarks(mainData.CalculationReference);
 inFile += AddLineWithMarks(DateTime.Now.ToString("MM/dd/yy").Replace('-', '/'));
 inFile += AddLineWithMarks(mainData.Designer);
 inFile += AddLineWithMarks(mainData.TransformerType );
 inFile += AddLine(mainData.TransformerSize + mainData.TransformerExtension); // added
 inFile += AddLineWithMarks(mainData.TransformerModel);
 inFile += AddLineWithMarks(mainData.MainStandard);
 inFile += AddLineWithMarks(mainData.SecondaryStandard);
 inFile += AddLine(mainData.Frequency);
 inFile += AddLine(mainData.HighestSystemVoltage);
 inFile += AddLine(mainData.PowerFrequencyInsulationLevel);
 inFile += AddLine(mainData.LightningInsulationLevel + "," + mainData.SwitchingInsulationLevel);
 inFile += AddLineWithMarks(vtData.RatedVoltage);
 inFile += AddLine(vtData.VoltageFactor);
 inFile += AddLineWithMarks(vtData.VoltageFactorTime);
 inFile += AddLineWithMarks(vtData.ThermalBurden);
 inFile += AddLine(mainData.MinimumAmbientTemperature + "," + mainData.MaximumAmbientTemperature + "," + mainData.AverageAmbientTemperature);
 inFile += AddLine(mainData.InsulatorCreepageDistance + "," + mainData.InsulatorFlashoverDistance);
 inFile += AddLineWithMarks(mainData.InsulatorType);
 inFile += AddLineWithMarks(mainData.InsulatorColor);
 inFile += AddLineWithMarks(mainData.PrimaryMarkings);
 inFile += AddLineWithMarks(emuData.IsExtendedNeutralTerminal);
 inFile += AddLineWithMarks(mainData.IsAutoMode);
 inFile += AddLineWithMarks(mainData.IsSpecialDesign);
 if (vtData.IsSpecialDesign)
 {
 string areEnabledMarkings = string.Empty;
 areEnabledMarkings += SpData.IsSelectionOfSecondaryTurns + ",";
 areEnabledMarkings += SpData.IsSelectionOfPrimaryWire + ",";
 areEnabledMarkings += SpData.IsSelectionOfSecondaryWire;
 inFile += AddLine(areEnabledMarkings);
 if (SpData.IsSelectionOfSecondaryTurns == "T")
 {
 inFile += AddLine(SpData.SecondaryTurnFirstWinding);
 }
 if (SpData.IsSelectionOfPrimaryWire == "T")
 {
 inFile += AddLineWithMarks(SpData.PrimaryWindingWireDimension);
 }
 if (SpData.IsSelectionOfSecondaryWire == "T")
 {
 foreach (var item in SpData.SecondaryWindingWireDimensions)
 {
 inFile += AddLineWithMarks(item.SecondaryMainWindingWireDimension);
 inFile += AddLineWithMarks(item.SecondaryTappedWindingWireDimension);
 }
 }
 }
 inFile += AddLineWithMarks(mainData.SerialNumberPrefix.ToString());
 inFile += AddLineWithMarks(mainData.SerialNumberSuffix.ToString());
 inFile += AddLineWithMarks(string.Empty); //XLNUMSUB ??
 inFile += AddLine(windingsData.Count.ToString());
 foreach (WindingData windingData in windingsData)
 {
 if (vtData.IsExtendedNumberOfClasses)
 {
 inFile += AddLineWithMarks("X");
 inFile += AddLine(windingData.NumberOfClassRequests);
 }
 inFile += AddLineWithMarks(windingData.TapMarking);
 inFile += AddLineWithMarks(windingData.EarthMarking);
 inFile += AddLineWithMarks(windingData.NominalVoltageTapped);
 inFile += AddLineWithMarks(windingData.NominalVoltageMain);
 inFile += extractBurdenAndAccuracyClassData(windingData.MainTerminalBurdenAndAccuracy);
 inFile += extractBurdenAndAccuracyClassData(windingData.MainTerminalOtherWindingBurdenAndAccuracy);
 if (vtData.IsExtendedNumberOfClasses)
 {
 inFile += extractBurdenAndAccuracyClassData(windingData.TappedTerminalBurdenAndAccuracy);
 inFile += extractBurdenAndAccuracyClassData(windingData.TappedTerminalOtherWindingBurdenAndAccuracy);
 }
 inFile += AddLineWithMarks(windingData.Fuse);
 foreach (string terminalMarking in windingData.TerminalMarking)
 {
 inFile += AddLineWithMarks(terminalMarking);
 }
 }
 string gyrStatus = string.Empty;
 gyrStatus += " ";//ALERT TOTAL 
 gyrStatus += translateGYRStatus(commonTransformerProperties.TransformerTypeAndSizeStatus);
 gyrStatus += translateGYRStatus(commonTransformerProperties.AverageAmbientTemperatureStatus);
 gyrStatus += translateGYRStatus(Insulator.Status);
 gyrStatus += " "; //Shell NO
 gyrStatus += " "; //Ipn
 gyrStatus += " "; //OMS
 gyrStatus += translateGYRStatus(secondaryWindings.FuseStatus);
 gyrStatus += " ";// Alert STATUS
 inFile += AddLineWithMarks(gyrStatus);
 return inFile;
 }
 catch (Exception ex)
 {
 throw ex;
 }
}
RubberDuck
31.1k6 gold badges73 silver badges176 bronze badges
asked Jan 22, 2016 at 18:39
\$\endgroup\$
1
  • 6
    \$\begingroup\$ First and foremost, quit using inFile as a string and instead use StringBuilder. \$\endgroup\$ Commented Jan 22, 2016 at 19:29

1 Answer 1

2
\$\begingroup\$

First, as mentioned in comments above, for efficiency you should be using StringBuilder when appending lots of strings repeatedly to make one big string.

The simplest answer I can give is to show you an example of how I might tackle the problem. The following uses a helper class FileContentWithMarks with the "know-how" for your custom formatting, and an instance of StringBuilder to build up the string.

What follows includes lots of second guesses based on the code snippet provided but hopefully it shows you a way to get started.

public enum ExportTransformerType
{
 Unknown,
 NonCapacitive,
 Capacitive
}
public string CreateLegacyInFile()
{
 try
 {
 var inFile = new FileContentWithMarks();
 // Here, I am assuming this is a way to derive the file type. YMMV.
 ExportTransformerType transformerType = DeriveTransformerType(mainData.TransformerType);
 inFile.AppendLineWithMarks(GetFolderLocation(transformerType));
 // Lines which are common beteween all types can go here verbatim.
 // You should also consider whether these can be organised into sub-methods
 // for a better overall picture of what's going on.
 inFile.AppendLineWithMarks(mainData.CalculationReference);
 inFile.AppendLineWithMarks(DateTime.Now.ToString("MM/dd/yy").Replace('-', '/'));
 inFile.AppendLineWithMarks(mainData.Designer);
 inFile.AppendLineWithMarks(mainData.TransformerType);
 inFile.AppendLine(mainData.TransformerSize +
 (transformerType == ExportTransformerType.Capacitive ?
 string.Empty
 :
 mainData.TransformerExtension));
 inFile.AppendLineWithMarks(mainData.TransformerModel);
 inFile.AppendLineWithMarks(mainData.MainStandard);
 // ... etc. More shared logic lines copied here
 if (transformerType == ExportTransformerType.Capacitive)
 {
 inFile.AppendLineWithMarks(capacitanceData.NominalIntermediateVoltage);
 }
 /// ... etc.
 inFile.AppendLineWithMarks(gyrStatus);
 // Finally, ask the helper to generate the complete string.
 return inFile.ToString();
 }
 catch (Exception ex)
 {
 throw ex;
 }
}
public ExportTransformerType DeriveTransformerType(string typeName)
{
 if (typeName == "CVT")
 return ExportTransformerType.Capacitive;
 if (typeName == "VT")
 return ExportTransformerType.NonCapacitive;
 return ExportTransformerType.Unknown;
}
public string GetFolderLocation(ExportTransformerType transformerType)
{
 string subFolder;
 switch (transformerType)
 {
 case ExportTransformerType.NonCapacitive:
 subFolder = "vt";
 break;
 case ExportTransformerType.Capacitive:
 subFolder = "cvt";
 break;
 default:
 subFolder = "unknown"; // or raise an exception if you prefer
 break;
 }
 return string.Format("{0}\\{1}\\{2}\\", dllWorkingDirectory, FactoryCode, subFolder);
}
// Wrapper for StringBuilder with specific line formatting/markings
public class FileContentWithMarks
{
 private StringBuilder _content = new StringBuilder();
 public void AppendLine(string line)
 {
 // The logic for how you format lines without marks can go here
 _content.AppendLine(line);
 }
 public void AppendLineWithMarks(string line)
 {
 // The logic for how you format lines with marks can go here
 _content.AppendLine("{" + line + "}");
 }
 public override string ToString()
 {
 return _content.ToString();
 }
}

Why bother defining this new helper class FileContentWithMarks? Let's look at the style of code required if inFile was a pure StringBuilder:

 inFile.Append(AddLineWithMarks(mainData.Designer));
 inFile.Append(AddLineWithMarks(mainData.TransformerType));

The more succinct form below is slightly easier to read and that in itself might be a good enough reason to use a helper...:

 inFile.AppendLineWithMarks(mainData.Designer);
 inFile.AppendLineWithMarks(mainData.TransformerType);

... but it also helps to encapsulate all the formatting logic within one place. This might benefit us in future when we need to add newer line categories (by defining new methods), or perhaps define different mark characters to be used (by adding a new class constructor that allows the default characters to be overridden).

Perhaps we will write another version of the FileContentWithMarks class, make both of them implement the same interface, but this version streams the lines both to a console and to a log file at once, doing away with the StringBuilder altogether. Then we can choose at runtime which class to instantiate without needing to change any of the instance method calls.

Designing in this way, separating out concerns, is good practice and beneficial to code reuse and maintenance.

answered Feb 18, 2016 at 20:10
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to CR! Good job on your first post! It would be nice if you could expand a bit on your idea of using this FileContentWithMarks class to wrap the StringBuilder, e.g. how it helps breaking down responsibilities and concerns, etc. \$\endgroup\$ Commented Feb 18, 2016 at 20:29

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.