Fixes and copyDynamicProperties #27
Conversation
| @@ -21,11 +21,8 @@ type DynamicObj() = | |||
| /// Gets property value | |||
| member this.TryGetValue name = | |||
There was a problem hiding this comment.
Does this fix #25 ? If so, please add a regression test
src/DynamicObj/DynamicObj.fs
Outdated
| /// Sets property value, creating a new property if none exists | ||
| member this.SetValue (name,value) = // private | ||
| // first check to see if there's a native property to set | ||
|
|
||
| match ReflectionUtils.tryGetPropertyInfo this name with | ||
| match ReflectionUtils.tryGetStaticPropertyInfo this name with |
There was a problem hiding this comment.
Now that you have this.TryGetStaticPropertyInfo, can you use it here instead of an external module?
There was a problem hiding this comment.
ReflectionUtils.tryGetStaticPropertyInfo is used although a method with the exact same name exists on the class. On other members, you used the instance method instead of the functions from ReflectionUtils
There was a problem hiding this comment.
For example, you used this.TryGetPropertyInfo in the Remove method instead of ReflectionUtils.tryGetPropertyInfo
| @@ -137,6 +164,23 @@ type DynamicObj() = | |||
| #endif | |||
| |> Seq.filter (fun kv -> kv.Key.ToLower() <> "properties") | |||
|
|
|||
| /// Copies all dynamic members of the DynamicObj to the target DynamicObj. | |||
| member this.CopyDynamicPropertiesTo(target:#DynamicObj, ?overWrite) = | |||
| let overWrite = Option.defaultValue false overWrite | |||
There was a problem hiding this comment.
There is also defaultArg, i vaguely remember that being better to use in these scenarios, but i do not remember why. Your call
There was a problem hiding this comment.
no i mean in general for determining default args for optional arguments
There was a problem hiding this comment.
I think here it doesn't matter.
In cases where the default expression is computationally expensive, it's worth to switch to an explicit match. But not sure whether Option.defaultValue and defaultArg even have different implementations.
#26