0

I have to implement classes to transfer files to USB (class USBTransfer) and over FTP (class FTPTransfer). Since both the classes use some common methods (like getting the filename, reading some parameters etc.) so, I have implemented those methods in an another class (class FileOperations). I have inherited both the classes (i.e. class USBTransfer and class FTPTransfer) from the class FileOperations.

class FileOperations
{
    protected void CommonMethod1();
    protected void CommonMethod2();
}

class USBTransfer : FileOperations
{ 

}

class FTPTransfer : FileOperations
{

}

PROBLEM: During the file transfer operations, I set different states (not using the state machine design pattern though). I want to use a ABSTRACT class for this purpose with the following structure.

abstract class FileTransferStateMachine
{
    //Public
    public enum FileTransferStates { Idle = 0, FileCopyingState = 1, SuccessfullyCopiedState = 2 }
    public static FileTransferStates CurrentState = FileTransferStates.Idle;

    abstract protected void IdleState();
    abstract protected void FileCopyingState();
    abstract protected void SuccessfullyCopiedState();
}

But in C# it is not allowed to have multiple inheritance.

QUESTION: I know that I can use interface. But you cannot have variables and in that case both of my classes (i.e. class USBTransfer and class FTPTransfer) will have their own variables for the following

public enum FileTransferStates { Idle = 0, FileCopyingState = 1, SuccessfullyCopiedState = 2 }
public static FileTransferStates CurrentState = FileTransferStates.Idle;

I want to reduce redundancy and want to force to have same variables/methods (hmm...same methods can be achieved by interface) for the state machine.

Question PART-2: I can transfer files to USB or FTP as mentioned above but both the transfer operations have some common states like IdleState, FileCopyingState or SuccessfullyCopiedState which have their corresponding methods (i.e. IdleState(), FileCopyingState() or SuccessfullyCopiedState()). I want to FORCE both the classes to implement these methods (i.e. IdleState(), FileCopyingState() or SuccessfullyCopiedState()). If any class forgets to implement any method then, I should get a compiler error. Basically, the FileTransferStateMachine should be an interface/abstract class whose methods should be overridden in USBTransfer and FTPTransfer classes (which are already inheriting another class called FileOperations).

5
  • 3
    Why use static here? Also why is FileTransferStates a nested type? Remove those things and turn it into an interface add another layer in the hierarchy chain. Commented Mar 16, 2017 at 13:20
  • FileTransferStates is static because this variable is read in an another class and by declaring it to be static, I don't need to have an object to access this variable in another class. Commented Mar 16, 2017 at 13:22
  • 4
    "... is static because this variable is read in an another class": ouch. Commented Mar 16, 2017 at 13:33
  • 1
    I agree with Igor, but additionally, surely a FileOperation is not a state, it has a state. So add a field ’private FileTransferStateMachine state’ to FileOperations, define some public getter, and you are good to go Commented Mar 16, 2017 at 13:40
  • @HugoRune: Could you please add a little sample structure for the USBFileTransfer and FTPFileTRansfer classes. I am feeling completely lost. I understood the answer of "Henk Holterman" below about using Composition. But I am confused how do I force the implementation of methods declared in FileTransferStateMachine (i.e. IdleState(), FileCopyingState() etc. ) ? I am confused how to properly utilize the Composition FileOperations as structured in "Henk Holterman". Commented Mar 16, 2017 at 15:37

2 Answers 2

3

Use composition (and avoid static state members) :

abstract class FileOperations
{
    protected void CommonMethod1();
    protected void CommonMethod2();

    protected FileTransferStateMachine Statemachine { get; set; }
}

Edit, regarding Part 2:

When you want to force the concrete classes to implement each of those methods then an interface IFileTransferStateMachine is exactly right.

You could also derive FileOperations from the StateMachine:
abstract class FileOperations: FileTransferStateMachine { }

but I would use an interface, it can be applied more granular.

Sign up to request clarification or add additional context in comments.

12 Comments

If I declare FileTransferStateMachine Statemachine as protected (and not static) then, how can I read the value of this variable in an another class (that class is just reading the value and publishing corresponding messages about the state).
The protected level was just a guess from my side, make it public. But you shouldn't need static, and you can't use it when 2 file operations are running simultaneously.
thanks, everybody is saying that static should not be used here. Could you please add some points about the negative effects of using a static in this case.
You seem to want to follow best practices with modeling and abstract classes. But a public static field is a global state nightmare. A practical point: that 'other class' could see what's happening but only for 1 file at a time. No way to find out what file. So give it a reference t the running FileOp in a decent way.
It's still a bad design. And it shouldn't be necessary.
|
0

Addressing both of the parts you say are a problem:

  • FileTransferStates can be moved out of the class, placed directly in the namespace (probably in its own file). Then it can be used by both implementations without being redefined

  • CurrentState can be made a non-static property (not field). Properties can be put on an interface, so they don't need an abstract class. Properties are highly recommended over fields for public members anyway, and as others have said, static isn't appropriate here.

As others have mentioned, another option is to use one of these two classes as a dependency instead of a base class. I.e. using composition instead of inheritance.

Comments

Your Answer

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.