Class design: file conversion logic and class design

Class design: file conversion logic and class design

This is pretty basic, but sort of a generic issue so I want to hear what people's thoughts are. I have a situation where I need to take an existing MSI file and update it with a few standard modifications and spit out a new MSI file (duplication of old file with changes).

I started writing this with a few public methods and a basic input path for the original MSI. The thing is, for this to work properly, a strict path of calls has to be followed from the caller:

var custom = CustomPackage(sourcemsipath); custom.Duplicate(targetmsipath); custom.Upgrade(); custom.Save(); custom.WriteSmsXmlFile(targetxmlpath); 

Would it be better to put all the conversion logic as part of the constructor instead of making them available as public methods? (in order to avoid having the caller have to know what the "proper order" is):

var custom = CustomPackage(sourcemsipath, targetmsipath); // saves converted msi custom.WriteSmsXmlFile(targetxmlpath); // saves optional xml for sms 

The constructor would then directly duplicate the MSI file, upgrade it and save it to the target location. The "WriteSmsXmlFile is still a public method since it is not always required.

Personally I don't like to have the constructor actually "do stuff" - I prefer to be able to call public methods, but it seems wrong to assume that the caller should know the proper order of calls?

An alternative would be to duplicate the file first, and then pass the duplicated file to the constructor - but it seems better to have the class do this on its own.

Maybe I got it all backwards and need two classes instead: SourcePackage, TargetPackage and pass the SourcePackage into the constructor of the TargetPackage?

c++ vector of class object pointers


Why can outer Java classes access inner class private members?
It may be just me, but the thought of a constructor doing all these things makes me shiver.

Namespace Class clash in C# - looking for suggestions
But why not provide a static method, which does all this:.
What is the difference between -> and :: in PHP?
public class CustomPackage  {      private CustomPackage(String sourcePath)      {          ... 

constructor as default argument
} public static CustomPackage Create(String sourcePath, String targetPath) { var custom = CustomPackage(sourcePath); custom.Duplicate(targetPath); custom.Upgrade(); custom.Save(); return custom; } }
The actual advantage of this method is, that you won't have to give out an instance of CustomPackage unless the conversion process actually succeeded (safe of the optional parts)..
Difference between implementing a class inside a .h file or in a .cpp file
Edit In C#, this factory method can even be used (by using delegates) as a "true" factory according to the Factory Pattern:.
Load method from current Controller
public interface ICustomizedPackage  {     ... 

c++ templated friend class
} public class CustomPackage: ICustomizedPackage { ...

} public class Consumer { public delegate ICustomizedPackage Factory(String,String); private Factory factory; public Consumer(Factory factory) { this.factory = factory; } private ICustomizedPackage CreatePackage() { return factory.Invoke(..., ...); } ...

and later:.
new Consumer(CustomPackage.Create); 


I'd go with your first thought: put all of the conversion logic into one place.

No reason to expose that sequence to users.. Incidentally, I agree with you about not putting actions into a constructor.

I'd probably not do this in the constructor, and instead do it in a separate converter method, but that's personal taste..


You're right to think that the constructor shouldn't do any more work than to simply initialize the object.. Sounds to me like what you need is a Convert(targetmsipath) function that wraps the calls to Duplicate, Upgrade and Save, thereby removing the need for the caller to know the correct order of operations, while at the same time keeping the logic out of the constructor.. You can also overload it to include a targetxmlpath parameter that, when supplied, also calls the WriteSmsXmlFile function.

That way all the related operations are called from the same function on the caller's side and the order of operations is always correct..


In such situations I typicaly use the following design:.
var task = new Task(src, dst); // required params goes to constructor task.Progress = ProgressHandler; // optional params setup task.Run(); 


I think there are service-oriented ways and object-oritented ways.. The service-oriented way would be to create series of filters that passes along an immutable data transfer object (entity)..
var service1 = new Msi1Service(); var msi1 = service1.ReadFromFile(sourceMsiPath); var service2 = new MsiCustomService(); var msi2 = service2.Convert(msi1); service2.WriteToFile(msi2, targetMsiPath); service2.WriteSmsXmlFile(msi2, targetXmlPath); 
The object-oriented ways can use decorator pattern..
var decoratedMsi = new CustomMsiDecorator(new MsiFile(sourceMsiPath)); decoratedMsi.WriteToFile(targetMsiPath); decoratedMsi.WriteSmsXmlFile(targetXmlPath); 

86 out of 100 based on 66 user ratings 566 reviews