Andre Merzky wrote:
Quoting [Ceriel Jacobs] (Nov 04 2009):
Andre Merzky wrote:
Quoting [Mathijs den Burger] (Nov 04 2009):
Hi all, ... 2. Removing SAGA attributes is permanent (AFAIK when talking to Ceriel), while a deleted dict item can be re-added latter Not sure I undestand. the following should be valid:
if ( ! advert.attribute_exists ("foo") ) { advert.set_attribute ("foo", "bar"); }
assert (advert.attribute_exists ("foo"), true);
advert.remove_attribute ("foo");
assert (advert.attribute_exists ("foo"), false);
advert.set_attribute ("foo", "bar");
assert (advert.attribute_exists ("foo"), true); Actually, I have an issue with this: I don't think this is valid.
Here is why, and it all depends on what the Saga specs mean by "exists". The notes at set_attribute say, a.o.: "the attribute is created, if it does not exist" and also "only some SAGA objects allow to create new attributes - others allow only access to predefined attributes. If a non-existing attribute is queried on such objects, a 'DoesNotExist' exception is raised". I take this to mean that ALL attributes mentioned in the SAGA specs DO exist, even if they don't have a value. Now, remove_attribute has as PostCond: "the attribute is not available anymore." I assume this means: "the attribute does not exist anymore", what else would it mean?
Absolutely correct so far, IMHO!
So, attribute_exists() returns false on a removed attribute, but then a call to set_attribute() gives a "DoesNotExist".
Here however you have two cases, as you actually quoted earlier:
"only some SAGA objects allow to create new attributes - others allow only access to predefined attributes. If a non-existing attribute is queried on such objects, a 'DoesNotExist' exception is raised".
FWIW, we call the objects which allow to create new attributes 'extensible'.
My point was, after remove_attribute, the attribute does not exist anymore, so, unless the object is 'extensible', the 'non-existing attribute' clause applies and a 'DoesNotExist' should be thrown.
So, it is
void saga::object::set_attribute (key, val) throws { switch ( extensible_ ) { case true: // always set attribs_[key] = val;
case false: // object is not extensible: only set attribute if it is one // of the allowed attributes if ( allowed_[key].count != 0 ) { // is a predefined attrib, can set attribs_[key] = val } else { // not extensible, not predefined: error throw saga::excdeption::does_not_exist ("object is not extensible, " "and " + key + " is not allowed"); } } }
Does that make sense to you?
It makes sense, but this is not what the specs currently say, IMHO.
From Mathijs, I understand that the C++ implementation has a different take on what "exists" means. I may even like that interpretation better, but I don't think it is supported by the current Saga specs.
I am obviously very biased, working with C++ mainly. But at the moment I don't see a clash with the spec.
Earlier however I discussed with Mathijs, and agree that the spec is unclear about which attributes are actually optional and which are mandatory. For example, 'Executable' on the job description is mandatory, all others on jd are optional. It is not extensible. Thus, IMHO, the following should hold:
saga::job::description jd;
// only lists one attrib: Executable assert (js.list_attributes ().count (), 1); // note: c++ is broken here as it shows an empty list - it only // complains when using the jd. I am fine with that one, too, but // the spec says differently, IMHO.
This assert is wrong, I think. Various other attributes "exist", but don't have a value. Would that be a reason for list_attributes to not list them? The specs say: Outputs: keys: existing attribute keys So, again, what does "exist" mean? I think that in a job_description, the attributes "Executable", "Arguments", and all other attributes mentioned in the specification of the job_description class "exist". Otherwise, set_attribute could never set them without throwing DoesNotExist.
// can set allowed attribs js.set_attribute ("PWD", "/tmp"); assert (js.list_attributes ().count (), 2);
// empty string is valid value js.set_attribute ("PWD", ""); assert (js.list_attributes ().count (), 2);
// removal removes key and val js.remove_attribute ("PWD", "/tmp"); assert (js.list_attributes ().count (), 1);
OK, it should certainly be one less than before. But now, the "PWD" attribute does no longer "exist" so another set_attribute("PWD", "...") throws DoesNotExist, according to the specs.
js.set_attribute ("foo", "bar"); // throws, not allowed
Basically, I think attribs should behave like a possibly pre-populated, a key-restricted hash table.
I agree! But the Saga specs currently say differently, I think.
So, lets see if that makes sense to you, and others, and if that actually matches the spec :-)
I don't think it matches the specs :-( Ceriel