-------- Original Message --------
Subject: Re: Standard String
Date: Fri, 15 Jul 2005 18:20:52 -0500
From: Andrew Johnson <[email protected]>
Organization: APS, Argonne National Laboratory
To: Jeff Hill <[email protected]>
CC: 'Marty Kraimer' <[email protected]>
References: <[email protected]>
Jeff,
I can see why you want the various methods that make up your
StringSegment sub-interfaces, but your design features are not all
appropriate to a concrete string class, especially inside iocCore.
The main thing that bothers me is your StreamPosition interface. The
position() and movePosition() methods implies that every StringSegment
includes an iterator into the string, which I assume is used by the
StreamRead and StreamWrite methods to indicate where their next access
will occur. It's only possible to have one such iterator per
StringSegment, so it's not possible for two threads to access the same
string simultaneously, even if they're both only reading from it.
I don't see how you can implement StringSegment::getChar() as a const
function if it changes the read position inside the string unless you
mark the position as mutable, which is lying to the compiler and hence
dangerous since calling getChar() *does* have an effect on the string
object. Similarly It's not clear whether your StringSegment::compare()
function changes the position or not for either StringSegment, although
my guess is that in the general case it has to for both.
If the V3 IOC used StringSegment instead of char* for storing record
names, that would means that as long as a lockset was locked, the CA UDP
task would not be able to complete a search where the record name it's
looking for is lower down the same hash bucket as a record in that
lockset (since the only record-related mutex we have in V3 is in the
lockset, so you'd have to take that before you'd be allowed to change
the current position of the name string).
I don't think that's acceptable, and I doubt that you would either. A
string is not an iterator and should not contain one, since it prevents
simultaneous access to two different parts of the same string, even from
inside the same function.
Your StringSegment is a string accessor interface with the methods that
you want, but it's not a general purpose string. It should be possible
to create a class that implements your StringSegment interface on top of
our final agreed string class, although I'm not sure whether that would
be the best or most efficient approach for your code.
To answer your questions about EpicsString:
Jeff Hill wrote:
3) Is EpicsString::createBuffer assigning a buffer to be used by the string?
It's confusing who manages the storage lifetime. Does ~EpicsString destroy
it with delete (making the class unusable should the buffer not be created
with new), or does the assigner destroy it? See also (3). From my
perspective, the implementation should decide how the string get allocated.
There are overloaded EpicsString::createBuffer() methods; if you pass in
an EpicsBufferCreator we just call it's create() method, otherwise we
punt to the EpicsBufferFactory::create() routine instead (which will
look up the EpicsBufferCreator for the type you requested and then call
its create() routine.
EpicsBufferCreator is a pure interface that you register with the
factory to tell it how to create an EpicsBuffer of your particular type.
Each specific EpicsBufferCreator is completely in charge of storage
management for all EpicsBuffer objects of its type, so the
~EpicsString() destructor calls the pbuffer->creator()->destroy() method.
4) The "EpicsBufferCreator *bufferCreator()" seems to require that
EpicsBufferCreator storage management is used. IMHO the core string
interface should not enforce a storage management solution. For example, in
CA I would like for the string to live in protocol buffers. I already have a
storage management solution based on a free lists template that I am quite
happy with. Furthermore, I might have a string constant implementation that
just references the string constant directly throwing an exception should
some author allow a non const ref to it.
EpicsBufferCreator is just an interface, but may not be the only
interface implemented by the derived class. You can create a class
which implements EpicsBufferCreator but that has additional create()
routines that you call directly with extra arguments. I can see that we
need a way to create an EpicsString using an EpicsBuffer that already
exists and contains data, good point. However every EpicsBuffer must
know who its EpicsBufferCreator is since that's the way we destroy the
buffer.
5) With regards to epicsString::get and epicsString::put my perception is
that the StreamPosition approach is cleaner compared to passing the extra
offset argument every time?
See my argument above regarding storing any kind of state related to a
scanning position inside the string object itself. The get() and put()
methods were provided as a convenience to someone who has a contiguous
buffer; they're not really essential, as you can use expose() instead
inside a loop to perform the same operation (see the EpicsBuffer::expose
description for details).
6) epicsString::hash might be a nice idea, but given that there are multiple
ways to hash a string, and given that we can hash a string using the
interfaces already present, then perhaps epicsString::hash is a pollution of
the core interface. I worry that eventually we will have two different
implementations of hash tables using epicsString. Will there be member
functions hash and hashImproved?
I accept this, we probably shouldn't have hash() in there at all.
7) This epicsString::isEqual taking an epicsBuffer as an argument is IMHO
not architecturally clean given that A) a string isn't being compared to a
string and B) the storage management and the string are getting mixed up
together.
That may be a typo, probably my fault. I think it should be taking an
EpicsString instead.
8) IMHO it is best to remain consistent with the convention of the C/C++
standard library and use size_t for the size of things.
I wondered about that, I'd accept that change everywhere.
- Andrew
--
Podiabombastic: The tendency to shoot oneself in the foot.
- Navigate by Date:
- Prev:
[Fwd: Re: Standard String] Marty Kraimer
- Next:
Re: [Fwd: RE: Standard String] Kay-Uwe Kasemir
- Index:
2002
2003
2004
<2005>
2006
2007
2008
2009
2010
2011
2012
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
- Navigate by Thread:
- Prev:
[Fwd: Re: Standard String] Marty Kraimer
- Next:
RE: Standard String Jeff Hill
- Index:
2002
2003
2004
<2005>
2006
2007
2008
2009
2010
2011
2012
2013
2014
2015
2016
2017
2018
2019
2020
2021
2022
2023
2024
| 
·
Home
·
News
·
About
·
Base
·
Modules
·
Extensions
·
Distributions
·
Download
·