Code-review question: should SimpleSerialShell::addCommand() be static?

[brucemack]

Great library!

It seems like the command list is shared between instances, since SimpleSerialShell::firstCommand is static. So therefore, should SimpleSerialShell::addCommand() be static as well for clarity purposes?

Either that, or should SimpleSerialShell:firstCommand be non-static? That would create the considerable advantage of allowing multiple independent instances of the shell processor to exist in the same program (connected to independent Streams, with independent command sets).

Thinking about that more, if the intention is to force a single instance per process (which is fine), perhaps the constructor should be made private so that a second instance can't be created? That would also improve clarity as to the intent.

Thanks,

Bruce

[philj404]

I think this is just something that I missed.

I set up the shell so it would just exist, and gave no instructions for creating an instance (so it was 'just magic'). Yes, it might be better to lock that down more explicitly.

The intent is that the shell is a Singleton object (exactly one instance exists), so there is not much difference between static and instance variables. I chose shell->addCommand(...) syntax because... I like how it looks. It better fits with Serial->println(), or Streaming syntax. (Even though SimpleSerialShell does not require the Streaming library, I find it quite useful.)

If someday there was a need for two different shell objects shell1 and shell2... (I'm already thinking of complications), I think I would still want just one shared (static) linked list. The overhead for managing two independent lists could take a lot of space.