Hi Jose, in c) I think return the previous value is more useful. in d) I think we can replace ! empty( xvalue) to ! hb_isnil(xvalue).
my 2 cents, Eduardo --- Em seg, 22/6/09, Jose F. Gimenez <jfgime...@wanadoo.es> escreveu: > De: Jose F. Gimenez <jfgime...@wanadoo.es> > Assunto: [xHarbour-developers] Big problem with TXML():SetAttribute() > Para: "xHarbour-Developers List" <xharbour-developers@lists.sourceforge.net> > Data: Segunda-feira, 22 de Junho de 2009, 7:34 > Hi, > > I've found a big problem (in terms of drastic behaviour > change) in the > method SetAttribute() of TXML class. This method was > written so: > > METHOD SetAttribute( cAttrib, xValue ) > INLINE ::aAttributes[ cAttrib ] := > xValue > > while now it is written as: > > > *-----------------------------------------------------------------------------* > METHOD SetAttribute( xAttrib, xValue ) > CLASS TXmlNode > > *-----------------------------------------------------------------------------* > Local lRet := .f. > > if ! empty( xAttrib ) .and. ! empty( xValue > ) <<<<<<<< NOTE HERE > > if HB_isString( xAttrib > ) // attribute name (key name) > > if HHasKey( > ::aAttributes, xAttrib) > <<<<<<<< AND HERE > > HSet(::aAttributes, xAttrib, xValue ) > lRet > := .t. > endif > > elseif HB_isNumeric( > xAttrib ) // attribute position (key ordinal > position) > > if Len( ::aAttributes ) > >= xAttrib > <<<<<<<< AND HERE > > HSetValueAt( ::aAttributes, xAttrib, > xValue ) > lRet > := .t. > endif > > endif > > endif > > RETURN lRet > <<<<<<<< AND HERE > > > There are several behaviour changes: > > a) Now, it's possible to set an attribute using its ordinal > position. That's > good, IMO. > > b) Now, it's NOT possible to set a new attribute using this > method. It only > change the previous value, but returns .f. if the attribute > name wasn't > exist. Very bad, IMO. > > c) The returned value is now .T. or .F., while before was > the setted value > itself, allowing it to be used in a chain. (F.e. xVal := > oXML:SetAttribute( "Attr", Value ) ). IMO, It's not good or > bad itself, but > it's a undesirable behaviour change. > > d) Now, if the desired value is "Empty()", then the > attribute is not setted. > It causes that values like .F. or 0 are not setted, while > they are perfectly > valid, IMO. > > > Please, may I change it back to the previous behaviour > (respecting a) if you > want, but reverting b), c) and d) ). Also, if someone need > a method to > change an attribute value without creating it, the actual > method can be > renamed as ChangeAttributeValue() or so. > > > Regards, > > Jose F. Gimenez > > > ------------------------------------------------------------------------------ > Are you an open source citizen? Join us for the Open Source > Bridge conference! > Portland, OR, June 17-19. Two days of sessions, one day of > unconference: $250. > Need another reason to go? 24-hour hacker lounge. Register > today! > http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org > _______________________________________________ > xHarbour-developers mailing list > xHarbour-developers@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/xharbour-developers > Veja quais são os assuntos do momento no Yahoo! +Buscados http://br.maisbuscados.yahoo.com ------------------------------------------------------------------------------ Are you an open source citizen? Join us for the Open Source Bridge conference! Portland, OR, June 17-19. Two days of sessions, one day of unconference: $250. Need another reason to go? 24-hour hacker lounge. Register today! http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org _______________________________________________ xHarbour-developers mailing list xHarbour-developers@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/xharbour-developers
