0

I have an application I'm writing that runs script plugins to automate what a user used to have to do manually through a serial terminal. So, I am basically implementing the serial terminal's functionality in code. One of the functions of the terminal was to send a command which kicked off the terminal receiving continuously streamed data from a device until the user pressed space bar, which would then stop the streaming of the data. While the data was streaming, the user would then set some values in another application on some other devices and watch the data streamed in the terminal change.

Now, the streamed data can take different shapes, depending on the particular command that's sent. For instance, one response may look like:

---RESPONSE HEADER---
HERE: 1
ARE: 2 SOME:3
VALUES: 4
---RESPONSE HEADER---
HERE: 5
ARE: 6 SOME:7
VALUES: 8
....

another may look like:

here are  some values
in   cols and  rows
....

So, my idea is to have a different parser based on the command I send. So, I have done the following:

public class Terminal 
{
   private SerialPort port;
   private IResponseHandler pollingResponseHandler;

   private object locker = new object();

   private List<Response1Clazz> response1;
   private List<Response2Clazz> response2;
   //setter omited for brevity
  //get snapshot of data at any point in time while response is polling.
   public List<Response1Clazz> Response1 {get { lock (locker) return new List<Response1Clazz>(response1); }
//setter omited for brevity
   public List<Response2Clazz> Response2  {get { lock (locker) return new List<Response1Clazz>(response2); }

   public Terminal()
   {
      port = new SerialPort(){/*initialize data*/}; //open port etc etc
          
   }
   
   void StartResponse1Polling()
   {
      Response1 = new List<Response1Clazz>();
      Parser<List<Response1Clazz>> parser = new KeyValueParser(Response1); //parser is of type T
      pollingResponseHandler = new PollingResponseHandler(parser);
      //write command to start polling response 1 in a task
   }

   void StartResponse2Polling()
   {
      Response2 = new List<Response2Clazz>();
      Parser<List<Response2Clazz>> parser = new RowColumnParser(Response2); //parser is of type T
      pollingResponseHandler = new PollingResponseHandler(parser); // this accepts a parser of type T
      //write command to start polling response 2
   }
   
   OnSerialDataReceived(object sender, Args a)
   {
      lock(locker){
       //do some processing yada yada
       //we pass in the serial data to the handler, which in turn delegates to the parser.
       pollingResponseHandler.Handle(processedSerialData);
     }       
   }
}

the caller of the class would then be something like

public class Plugin : BasePlugin
{
  public override void PluginMain()
  {
    Terminal terminal = new Terminal();
    terminal.StartResponse1Polling();
    //update some other data;
    Response1Clazz response = terminal.Response1;
    //process response
    //update more data
    response = terminal.Response1;
    //process response
    //terminal1.StopPolling();
    
  }
}

My question is quite general, but I'm wondering if this is the best way to handle the situation. Right now I am required to pass in an object/List that I want modified, and it's modified via a side effect. For some reason this feels a little ugly because there is really no indication in code that this is what is happening. I am purely doing it because the "Start" method is the location that knows which parser to create and which data to update. Maybe this is Kosher, but I figured it is worth asking if there is another/better way. Or at least a better way to indicate that the "Handle" method produces side effects.

Thanks!

gfree
  • 479
  • 7
  • 16

1 Answers1

-1

I don't see problems in modifying List<>s that are received as a parameter. It isn't the most beautiful thing in the world but it is quite common. Sadly C# doesn't have a const modifier for parameters (compare this with C/C++, where unless you declare a parameter to be const, it is ok for the method to modify it). You only have to give the parameter a self-explaining name (like outputList), and put a comment on the method (you know, an xml-comment block, like /// <param name="outputList">This list will receive...</param>).

To give a more complete response, I would need to see the whole code. You have omitted an example of Parser and an example of Handler.

Instead I see a problem with your lock in { lock (locker) return new List<Response1Clazz>(response1); }. And it seems to be non-sense, considering that you then do Response1 = new List<Response1Clazz>();, but Response1 only has a getter.

xanatos
  • 109,618
  • 12
  • 197
  • 280
  • Yes, whoops that's what I get for writing this up by memory. I have an initializer to a list not a new list in the getter. Also, I put a comment in there that says I omited the setter for brevity. I should have been more clear. Thanks! – gfree Jan 25 '21 at 05:01