Closed Bug 717356 Opened 13 years ago Closed 13 years ago

String classes should achieve type genericity using templates, not macros

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: bjacob, Unassigned)

References

Details

Attachments

(1 file)

Attached patch templates++, macros-- (deleted) — Splinter Review
See e.g. nsTSubstring_CharT in nsTSubstring.h. I'm attaching a patch that's how far I went before deciding to stop this work as I'm not sure I understand everything that I should. The basic design choice that I can see here, is whether e.g. nsString and nsCString should just be typedefs for specializations of the same nsTString template, or should they rather be classes of their own, inheriting specializations of a common base class template nsTString (maybe call it nsTStringBase). The present patch implements the typedef approach. For example, nsTString_CharT is replaced by a nsTString class template and then nsString is a typedef for nsTString<PRUnichar>. The advantage of this typedef approach is that it avoids uselessly increasing the number of types at hand. It's easy to wrap one's head around the notion that nsString is a typedef for nsTString<PRUnichar>, it just makes sense. The downside is that it makes it harder to have specific methods in only one of nsString and nsCString and not in the other. That's where I stopped: if you try compiling with this patch, you'll get compile errors as nsStringObsolete.cpp tries to define methods specifically in nsString / in nsCString, and I wasn't sure what to do about that. In itself it sounds legitimate to have specific methods, but as a matter of fact, the only such type-specific methods we have are "obsolete" (whatever that means --- they're still enabled). By default I'm leaving it here, unless someone in the know thinks this is worth carrying on and explains how to best deal with these obsolete methods.
Darin tried this originally and moved "back" to the templates because the minor API differences and the somewhat major implementation differences meant that you had to basically specialize everything, which removes most of the benefit. I understand that the templates give somewhat better code readability/searchability, but I don't think it's worth the effort in general to change things yet again.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → INCOMPLETE
There's something that I don't understand here: while not everything can be made generic, I was thinking that what is made generic with macros could equally be made generic with templates. That part works nicely in above patch. Anyway, don't want to steal more of your time.
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: INCOMPLETE → ---
Oops, I didn't mean to reopen this bug.
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: