axis-c-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From lah...@opensource.lk
Subject RE: STL elimination - Suggestions for string
Date Fri, 04 Jun 2004 09:14:27 GMT
Hi Kenneth

Thanks for Your comments

Lahiru
> Comments inline.
>
>       1     #if !defined(_ASTRING_H____OF_AXIS_INCLUDED_)
>       2     #define _ASTRING_H____OF_AXIS_INCLUDED_
>       3
>       4     #include <string.h>
>       5
>       6
>       7     class Astring
>       8     {
>       9
>      10
>      11     public:
>      12         static const int npos;
>      13         Astring();
>      14         Astring(const char* pcCharacter);
>      15         Astring(const char ccValue);
>      16         ~Astring();
>      17
>      18         bool operator ==(const Astring & AstrParam);
>      19         bool operator ==(const char * cpRightVal);
>      20         void operator =(const char* pcParam);
>      21         void operator =(const Astring& objAstring);
>      22         void operator =(const char cValue);
>      23         const char* c_str() const;
>      24     //  Astring operator +(const Astring & rAstrRight);
>      25     //  Astring operator +(const char * cpValue);
>      26         friend Astring operator+(Astring & rAstrRight,const char *
> cpValue);
>      27         friend Astring operator+(char * cpValue,const Astring &
> rAstrRight);
>      28         friend Astring operator+(Astring & rAstrLeft,Astring &
> rAstrRight);
>      29         Astring substr(int pos=0,int inelements=npos)const;
>      30         int length()const;
>      31         int size()const;
>      32         void operator +=(const Astring & rAstrRight);
>      33         void operator +=(const char * ccpRightchar);
>      34         void operator +=(const char ccValue);
>      35         bool empty() const;
>      36         int find(Astring & rAstrRight,int istart=0)const;
>      37         int find(const char cFind,int istrat=0)const;
>      38         int find(const char* ccpFind,int istart=0)const;
>      39         int find_first_of(Astring & rAstrRight,int istart=0)const;
>      40         int find_first_of(const char* cpValue,int istart=0)const;
>      41         int rfind(char * cpValue,int ilastval=npos)const;
>      42         int rfind(const char cValue,int ilastval=npos)const;
>      43         char at(int ipos)const;
>      44         Astring& append(const char * cpValue);
>      45         Astring& append(Astring & rAstrRight);
>      46         char & operator [](int iIndex);
>      47
>      48     /*
>      49         friend Astring operator+(Astring & rAstrRight,const char *
> cpValue)
>      50         {
>      51             Astring * AstrTemp=new Astring(rAstrRight.m_pcValue);
>      52             *AstrTemp += cpValue;
>      53             return *AstrTemp;
>      54         }
>      55
>      56         friend Astring operator+(char * cpValue,const Astring &
> rAstrRight)
>      57         {
>      58             Astring * AstrTemp=new Astring(rAstrRight.m_pcValue);
>      59             *AstrTemp += cpValue;
>      60             return *AstrTemp;
>      61         }
>      62
>      63         friend Astring operator+(Astring & rAstrLeft,Astring &
> rAstrRight)
>      64         {   Astring * AstrTemp=new Astring(rAstrLeft.m_pcValue);
>      65             *AstrTemp += rAstrRight;
>      66             return *AstrTemp;
>      67         }
>      68         */
>      69     private:
>      70         char* m_pcValue;
>      71
>      72     };
>      73
>      74
>      75
>      76     #endif
>
>       1     #include <malloc.h>
>       2     #include <stdlib.h>
>       3     #include <iostream>
>       4     #include "Astring.h"
>       5
>       6     const int  Astring::npos=-1;
>       7
>       8     /*
>       9      * Constructor
>      10      */
>      11     Astring::Astring() {
>      12         m_pcValue = '\0';
>      13     }
>      14
>      15     Astring::Astring(const char* pcCharacter)
>      16     {
>      17         if(pcCharacter=="")
>      18             m_pcValue = '\0';
>      19         else
>      20         {
>      21             m_pcValue = (char*)malloc(strlen(pcCharacter)+1);
>      22             strcpy(m_pcValue, pcCharacter);
>      23         }
>      24     }
>      25
>      26     Astring::Astring(const char ccValue)
>      27     {
>      28         m_pcValue=(char*)malloc(2);
>      29         *m_pcValue=ccValue;
>      30         *(m_pcValue+1)='\0';
>      31     }
>      32
>      33     Astring::~Astring() {
>      34         if (m_pcValue)
>      35         {
>      36             free(m_pcValue);
>      37             m_pcValue = NULL;
>      38         }
>      39     }
>
> You need a copy constructor here.  Otherwise, you will have
> memory corruption, because you will be freeing memory twice.
>
>      40
>      41     bool Astring::operator ==(const Astring &AstrParam)
>      42     {
>      43
>      44         int result=strcmp(m_pcValue ,AstrParam.m_pcValue );
>      45         if(result==0)
>      46         {
>      47             return true;
>      48         }
>      49         else
>      50         {
>      51             return false;
>      52         }
>      53
>      54     }
>
> I think this will crash on empty string.
>
>     Astring a(""), b("foo"), c;
>     a == b;  // a.m_pcValue is NULL.
>     c == b;  // c.m_pcValue is NULL.
>
>      55
>      56     bool Astring::operator ==(const char * cpRightVal)
>      57     {
>      58          int result=strcmp(m_pcValue ,cpRightVal );
>      59         if(result==0)
>      60         {
>      61             return true;
>      62         }
>      63         else
>      64         {
>      65             return false;
>      66         }
>      67
>      68
>      69     }
>      70
>      71
>      72     void Astring::operator = (const char* pcParam)
>      73     {
>      74         m_pcValue = (char*)malloc(strlen(pcParam)+1);
>      75         strcpy(m_pcValue, pcParam);
>      76     }
>
> What kind of semantics do you want for empty string.  It
> seems from the constructor that you want empty string to be
> NULL.  In that case, you should probably check here too.
> Otherwise, you would have this.
>
>     A a(""), b;
>     b = "";
>     // At this point, a.m_pcValue is NULL, but b.m_pcValue
>     // is an empty string.  This is probably not what is
>     // expected.
>
>      77
>      78     void Astring::operator = (const Astring& objAstring)
>      79     {
>      80         m_pcValue = (char*)malloc(strlen(objAstring.m_pcValue)+1);
>      81         strcpy(m_pcValue, objAstring.m_pcValue);
>      82
>      83     }
>      84
>      85     void Astring::operator =(const char cValue)
>      86     {
>      87         m_pcValue=(char*)malloc(2);
>      88         *m_pcValue=cValue;
>      89         *(m_pcValue+1)='\0';
>      90     }
>      91     const char* Astring::c_str() const
>      92     {
>      93         if (m_pcValue != NULL)
>      94         {
>      95             return m_pcValue;
>      96         }
>      97
>      98         else
>      99         {
>     100             return "";
>     101         }
>     102     }
>     103
>     104
>     105     Astring Astring::substr (int iPos,int inelements)const
>     106     {
>     107         int ilastpos;
>     108
>     109         if (inelements!=npos)
>     110         {
>     111             ilastpos=iPos+inelements-1;
>     112         }
>     113
>     114         if ((inelements==npos) || (ilastpos>length()-1))
>     115         {
>     116             ilastpos=length()-1;
>     117
>     118         }
>     119
>     120
>     121
>     122         char cTemp;
>     123         char* pcSubstring = new char[ilastpos-iPos+2];
>     124
>     125         for(int iIndex=0;iIndex<(ilastpos-iPos+1);iIndex++)
>     126         {
>     127             cTemp=*(m_pcValue+iPos+iIndex);
>     128             pcSubstring[iIndex]=cTemp;
>     129
>     130         }
>     131
>     132         pcSubstring[ilastpos-iPos+1]='\0';
>     133         Astring * as = new Astring(pcSubstring);
>     134         delete pcSubstring;
>     135         return *as;
>     136
>     137     }
>     138     int Astring::length ()const
>     139     {
>     140         if(m_pcValue!=NULL)
>     141         {
>     142             int ilen=strlen(m_pcValue);
>     143             return ilen;
>     144         }
>     145
>     146         else
>     147             return 0;
>     148     }
>     149
>     150     int Astring::size()const
>     151     {
>     152         return length();
>     153     }
>     154
>     155     void Astring::operator += (const Astring & rAstrRight)
>     156     {
>     157         if(m_pcValue!=NULL)
>     158         {
>     159
> m_pcValue=(char*)realloc(m_pcValue,strlen(m_pcValue)+strlen(rAstrRight.m_pcValue)+1);
>     160             strcat(m_pcValue,rAstrRight.m_pcValue );
>     161         }
>     162         else
>     163         {
>     164             int iLen=rAstrRight.length ()+1;
>     165             m_pcValue=(char*)malloc(iLen);
>     166             strcpy(m_pcValue,rAstrRight.m_pcValue );
>     167         }
>     168     }
>     169
>     170     void Astring::operator +=(const char * ccpRightchar)
>     171     {
>     172         if(m_pcValue!=NULL)
>     173         {
>     174
> m_pcValue=(char*)realloc(m_pcValue,strlen(m_pcValue)+strlen(ccpRightchar)+1);
>     175             strcat(m_pcValue,ccpRightchar );
>     176         }
>     177
>     178         else
>     179         {
>     180             int iLen=strlen(ccpRightchar)+1;
>     181             m_pcValue=(char*)malloc(iLen);
>     182             strcpy(m_pcValue,ccpRightchar );
>     183         }
>     184     }
>     185
>     186     void Astring::operator +=(const char ccValue)
>     187     {
>     188         if(m_pcValue != NULL)
>     189         {
>     190             int iLength=strlen(m_pcValue);
>     191
> m_pcValue=(char*)realloc(m_pcValue,(sizeof(m_pcValue)+2));
>     192             *(m_pcValue+iLength)=ccValue;
>     193             *(m_pcValue+iLength+1)='\0';
>     194         }
>     195
>     196         else
>     197         {
>     198             m_pcValue=(char*)malloc(2);
>     199             *m_pcValue=ccValue;
>     200             *(m_pcValue+1)='\0';
>     201         }
>     202     }
>
> If you are going to be creating a lot of strings by
> repeatedly appending to them:
>
>     Astring s;
>     for (...) {
> 	s += some_var;
>     }
>
> then you will want to keep a length and capacity, and expand
> the capacity exponentially.  This is to avoid O(N^2)
> behavior.
>
>     203
>     204
>     205     /*Astring Astring::operator +(const Astring & rAstrRight)
>     206     {
>     207         Astring * AstrTemp=new Astring(m_pcValue);
>     208         *AstrTemp += rAstrRight;
>     209         return *AstrTemp;
>     210     }
>     211
>     212     Astring Astring::operator+(const char * cpValue)
>     213     {
>     214         Astring * AstrTemp=new Astring(m_pcValue);
>     215         *AstrTemp += cpValue;
>     216         return *AstrTemp;
>     217     }*/
>     218
>     219     bool Astring::empty () const
>     220     {
>     221         return m_pcValue=='\0';
>     222
>     223     }
>
> Do you need to distinguish between an "null" Astring (i.e.,
> it has no value) and an empty Astring?
>
>     224
>     225     int Astring::find (Astring & rAstrRight,int istart)const
>     226     {
>     227         int iLenrAstrRight=rAstrRight.length();
>     228         int istop=(length()-iLenrAstrRight) + 1;
>     229
>     230         for(int i=istart; i<istop; i++)
>     231         {
>     232             Astring Astrtext=substr(i,iLenrAstrRight);
>     233             if (Astrtext==rAstrRight)
>     234             {
>     235                 return i;
>     236             }
>     237
>     238         }
>     239
>     240         return -1;
>     241
>     242     }
>     243
>     244     int Astring::find(const char* ccpFind,int istart)const
>     245     {
>     246         int iLenrAstrRight=strlen(ccpFind);
>     247         int istop=(length()-iLenrAstrRight) + 1;
>     248
>     249         for(int i=istart; i<istop; i++)
>     250         {
>     251             Astring Astrtext=substr(i,iLenrAstrRight);
>     252             if (Astrtext==ccpFind)
>     253             {
>     254                 return i;
>     255             }
>     256
>     257         }
>     258
>     259         return -1;
>     260     }
>     261
>     262     int Astring::find_first_of (Astring & rAstrRight,int
> istart)const
>     263     {
>     264         int ipos;
>     265         Astring Astrtext=substr(istart,(length()-istart));
>     266         ipos=strcspn(Astrtext.m_pcValue,rAstrRight.m_pcValue );
>     267
>     268         return (ipos+istart);
>     269
>     270     }
>     271
>     272     int Astring::find_first_of(const char* cpValue,int
> istart)const
>     273     {
>     274         int ipos;
>     275         Astring Astrtext=substr(istart,(length()-istart));
>     276         ipos=strcspn(Astrtext.m_pcValue,cpValue );
>     277
>     278         return (ipos+istart);
>     279
>     280     }
>     281
>     282
>     283     char Astring::at (int ipos)const
>     284     {
>     285         return (*(m_pcValue+ipos));
>     286     }
>
> As a defensive programming measure, it might not be a bad
> idea to put an assert() here for range-checking.  The
> assert() will not be compiled in when you define NDEBUG.
> Also note that this will crash if m_pcValue is NULL.
>
> On Thu, 3 Jun 2004 lahiru@opensource.lk wrote:
>
>> hi all,
>>
>>  Please have a look at the Astring class implemented below and provide
>> your comments. The  files are attached with this mail.
>>
>> rgds,
>> Lahiru
>>
>> > At 10:13 AM 5/6/2004 +0530, you wrote:
>> >>What I would suggest -
>> >>1. expose a small set of classes and make the interface stl-free
>> >>2. in your internal classes you can still use stl
>> >>
>> >>Why do you want to remove stl all-together?
>> >>
>> >> > -----Original Message-----
>> >> > From: Kenneth Chiu [mailto:chiuk@cs.indiana.edu]
>> >> > Sent: Thursday, May 06, 2004 9:41 AM
>> >> > To: Apache AXIS C Developers List
>> >
>>
>
>


Mime
View raw message