views:

93

answers:

4

I am not sure if what I want to do breaks Object oriented guidelines or not so I will explain what I am doing and hopefully you guys can show me a better way if I am wrong. I tried asking this question before but I gave a poor example so I think it just caused more confusion.

So I have a main class, USBCommunicator. The constructor takes a Product ID of the type of device that you want to talk to. The USBCommunicator class also has a property for a specific serial number to talk to. USBCommunicator has OpenConnection and CloseConnection methods which will open or close a data stream to transfer data between the USB device and the PC.

To send data across the stream I want the USBCommunicator to be able to create an instance of a Report class, set some parameters such as timeouts, ReportID's etc, and then call a Send() method of the Report class to actually send the data. I don't think any class other than USBCommunicator should be able to create an instance of the Report class. (For example, a Boat lass shouldn't be able to create instances of CarDoor class because boat can't have a car door.) Finally, I was originally thinking that the Report class should be able to have access to the members of USBCommunicator but I guess that is not true. If USBCommunicator opens the stream the the device, all Report really needs is a parameter passed in that is a reference/handle to the open stream. But what form should that stream be that it would allow it to be passed by the high level application? a public property? That doesn't seem quite right.

So here is what I have so far...

namespace USBTools
{
    class HighLevelApplication
    {
        void main()
        {
            USBCommunicator myUSB = new USBCommunicator("15B3");
            myUSB.SerialNumber = "123ABC";
            myUSB.OpenConnection();
            myUSB.Report ReportToSend = new myUSB.Report(//how do I pass the stream here?);
                //It would be nice if I didn't have to pass in the stream because the stream shouldn't
                //be publicly available to the HighLevelApplication class right?
            ReportToSend.ReportID = 3;
            ReportToSend.Timeout = 1000;
            ReportToSend.Data = "Send this Data";
            ReportToSend.Send();
        }
    }

    class myUSB
    {
        myUSB(string PID)
        {
            //...
        }

        // public SerialNumber property

        // private stream field ???

        // public OpenConnection and CloseConnection methods

        class Report
        {
            Report(stream StreamToUse)
            {
                //...
            }
            Send()
            {
                //send the data
            }
        }
    }
}
+1  A: 

You do not explicitly access private members, you provide public properties that return the private variables from your Report class.

Also as long as your report class is marked public, you can do:

Report r = new Report();

Within your main USBCommunicator class. But never expose member variables as public, they should be private within Report but you should include public properties to access those private members.

JonH
A: 

Why don't you pass the whole class (either by ref or copy)?

myUSB.Report ReportToSend = new myUSB.Report(ParentClassWithStream);

myUSB.Report must have a private member to hold the reference.


...
 class Report
        {
            ParentClassWithStream PC
            Report(ParentClassWithStream p)
            {
                PC = p

                //...
            }
            Send()
            {
                //send the data
            }
        }
...

masterLoki
Doesn't this break object-oriented guidelines?
Jordan S
If your parent class respects encapsulation then no, since you'll be using your access methods to get the information you need. It's just a class referring to another one
masterLoki
+1  A: 

The following would do what you want. The idea is to seperate the interface for the Report class from the actual implementation. That lets you create an implementation class that only your USBCommunicator object can create.

public abstract class Report
{
   protected Report() { }

   public int ReportID {get; set;}
   public int Timeout {get; set;}
   public string Data {get; set; }
   public abstract void Send();
}

public class USBCommunicator 
{ 
    private Stream m_stream;

    public USBCommunicator (string PID) 
    { 
        //... 
    } 

    //Callers create new report objects via a method instead of directly using 'new'
    public Report CreateReport()
    {
        return new ReportImpl(m_stream);   
    }

    //Provides the implementation of the abstract methods of the Report class.
    private class ReportImpl : Report
    {
        private Stream m_stream;

        public ReportImpl(Stream stream)
        {
           m_stream = stream;
        }

        public void override Send()
        {
           //put implementation of Send here.
        }
    }
}

Your high level app then becomes:

class HighLevelApplication        
{        
    void main()        
    {        
        USBCommunicator myUSB = new USBCommunicator("15B3");        
        myUSB.SerialNumber = "123ABC";        
        myUSB.OpenConnection();        

        Report reportToSend = myUSB.CreateReport();

        reportToSend.ReportID = 3;        
        reportToSend.Timeout = 1000;        
        reportToSend.Data = "Send this Data";        
        reportToSend.Send();
    }        
} 
alabamasucks
So where does my report class sit and what prevents the HighLevelApplication from doing this Report reportToSend = new Report(); ? That would be bad because it wouldn't have a stream right?
Jordan S
Report is abstract and Send is not implemented, so you wouldn't be able to do anything with it until you inherit Report and implement Send somewhere else.
s_hewitt
+3  A: 

Since USBCommunicator manages all the important resources (including the stream's lifetime), applications should call USBCommunicator.Send, not Report.Send:

public class USBCommunicator {
    public void Send(Report report) {

        // If it's appropriate, this method can also Open 
        // and Close the stream so callers don't have to.

        report.Send(this.stream);
    }
}

Next, make Report.Send internal so it isn't part of the public API, and applications can do this:

public void main(string[] args) {
        USBCommunicator myUSB = new USBCommunicator("15B3");
        myUSB.SerialNumber = "123ABC";
        myUSB.OpenConnection();

        Report report = new Report(3, 1000, "Send this Data");
        myUSB.Send(report);
}

I don't think any class other than USBCommunicator should be able to create an instance of the Report class.

Your current design doesn't prevent this - indeed, your sample application is creates an instance of the Report class. If you really want to hide the report class, you should make it private to USBCommunicator and push its attributes out to the communicator's interface like this:

// USBCommunicator
public void Send(int reportID, int timeout, string data) {
    Report report = new Report(reportID, timeout, data);
    report.Send(this.stream);
}
Jeff Sternal
+1 - What I posted mentioned properties but this is good advice.
JonH
Well done. Thank you.
Jordan S