-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Adding Inserters for the Print Class #5002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@tomstewart89 , thanks for the PR, could you squash the commits into a single one and force-push to the same branch so we can get a cleaner history? |
Streaming capability if implemented fully is quite a useful feature, however, I think as it stands it is quite limited as it does not allow any customization of the specifics like integer base or decimal precision, which limits it to trivial printing only. A sketch of mild complexity will end up having to contain a combination of methods which might end up being more elaborated than using one method or the other. And the inverse is not available (reading from a Stream using streaming operators). This is the reason I created the OStream and IStream(Input from Stream based objects) functionality for my PrintEx lib, easily available from the Library manger. It not only contains the standard modifiers (hex, dec, precision, repeat, etc), but can also stream to/from memory spaces like RAM and EEPROM & Flash (read only). However, I would like to stress, if this does go ahead the functions most definitely should be changed to member functions, not global overloads. The global methods are only needed now as they currently aren't part of the Print class. The importance of this is: as member functions, more advanced functionality like my lib will still work as the base class member functions will be able to be overridden. |
Removed swap files Update bundled Temboo library to 1.1.6 Update sha for Temboo 1.1.6 fix for #4993 Update revision log
@Chris--A makes some fair points. I wasn't aware of the PrintEx library and without a doubt it has much nicer features than the change I'm suggesting here. It's also the case that defining an operator<< for Print causes PrintEx to error and I'm not looking to disrupt a perfectly good library. As far as I can tell though, PrintEx errors due to ambiguity brought about by inheriting from both OStreamBase and Print, both of which would have definitions for operator<<. I can't see how making the Print operator a member function would resolve that, but perhaps @Chris--A could suggest a solution. Until then I don't mind shelving the request, it's not an urgent feature or anything. At the same time though, a few of the classes in my libraries define their own inserters for Print which to me is a more standard and slightly faster option than subclassing Printable. It'd be great to be able to print POD's in the same way. |
A member function with the same name in a derived class is overridden (replaced). You'd explicitly have to call it by casting the this pointer inside the class, or explicitly include it using a |
@@ -97,6 +97,7 @@ typedef const void* uint_farptr_t; | |||
#define memcmp_PF(s1, s2, n) memcmp((s1), (s2), (n)) | |||
|
|||
#define sprintf_P(s, f, ...) sprintf((s), (f), __VA_ARGS__) | |||
#define snprintf_P(s, f, ...) snprintf((s), (f), __VA_ARGS__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated to the PR.
Please refer to #4996
Ah yeah that's my mistake, I merged the branch with upstream before I made the pull request. I'm still getting the hang of using git so apologies for that. If there's something I can do to fix it, please let me know. As for making the operator overload a member function, I completely agree. Having said that, and correct me if I'm wrong, if you define a member function of the same name in a derived class it will overload the one in the base class but not replace it. The result will be two functions accessible from the derived class whose parameters differ by the type of the 'this' pointer which they are passed implicitly. Although, if the base class function is virtual then absolutely, it'll be replaced, and only member functions can be virtual so if that's what you meant then I agree. Even if the operator is defined as virtual though, there will still exist two operator<< functions which accept a pointer to a base of PrintEx so the choice of which to use on a call to PrintEx::operator<< will be ambiguous. As you say though, you can get around this by explicitly defining an operator<< for the PrintEx class and in it's implementation point the compiler to one of the two base class operators using a cast or scope resolution. |
In this instance no, like I mentioned before, it is overriding. The base class member function is hidden and not accessible through the derived class. It is only an overload when the derived member has a different signature (parameters, CV qualification). If the functions have the same signature, the base class member is hidden. This is overriding, as the templates will resolve to the same signature, therefore hiding the base class function. Name hiding can occur in may forms, e.g: struct A{
char x;
void print(){ Serial.print("A::print"); }
};
struct B : A{
float x;
void print(){ Serial.print("B::print"); }
};
B b;
//...
b.x; // << x is a float.
b.print(); // << Prints "B::print" Still, I do not think this is an overly useful feature as it stands. To get the desired output, people will end up using other methods (Strings, c-string methods) to achieve the formatting they need just so they can use an insertion. Which will probably end up with more code than simply calling the pre-provided Print functions which allow custom formatting. It is easy enough for someone to include the Streaming lib, or my PrintEx lib to get Streaming capability and formatting features that come with them. |
Oh so it is! I hadn't heard of name hiding, that's really interesting. As for the pull request, yeah that's fine, let's not worry about it.Thanks for the feedback though |
Hello,
I was thinking it'd be nice to have an insertion operator for the Print class as suggested here:
http://playground.arduino.cc/Main/StreamingOutput
I know it's defined in that streaming library but it's such a common and convenient bit of syntax that I thought it'd be worth including in the core.
Let me know what you think!