On Saturday, 21 July 2018 at 05:59:37 UTC, Manu wrote:
[...]
Let me give a concrete example of one of the situations Jonathan
is describing. Consider the following code:
struct Secret
{
public:
string key;
}
/* ... */
genrateRandomKey(ref string key) {
key = /* some code to actually generate the key */
}
Secret secret;
generateRandomKey(secret.key);
Now somebody else working on the project who sees the definition
of Secret might think "Having public access to member variables
is bad, so lets use property methods instead. This even allows us
to do some contract checks!" and he changes the definition of
Secret to the following:
struct Secret
{
private:
string _key;
public:
string key() @property const
{
return this._key;
}
void key(string key) @property
in
{
assert(key.length == 256)
}
do
{
this._key = key;
}
}
Now with DIP 1016, the use of generateRandomKey silently "fails",
i.e. secret._key will not be changed by it, which in this case is
a big problem as the key is still default initialized!
Of course one might argue that genrateRandomKey should not take
its argument by reference and rather just return the key instead.
But in my experience, there is quite a lot of code like this out
there (e.g. in order to avoid copying, string is probably not the
best example here...).
In one of your earlier answers you argued, that in cases like
this, the @property function should probably have returned by
reference and that not doing so is the real bug. Do you also
think this is true in this case? I don't think so. One reason is
for example, that you can not put contracts on your setter
@property method then...
In my opinion, there is no bug with the definition of the
@property methods here. The bug only arises through interactions
with functions which take its parameters by reference.
Do you think this example in contrived? If yes, why?
