# Code Review: YamlFixes

**URL:** <https://www.ruby-forum.com/t/code-review-yamlfixes/143853>\
**Category:** IronRuby\
**Created:** [July 23, 2008, 10:36am UTC](https://www.ruby-forum.com/t/code-review-yamlfixes/143853 "2008-07-23T10:36:48Z")\
**Posts on this page:** 6\
**Page:** 1

<div class="post-metadata">

**Author:** ![Oleg\_T](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/o/f19dbf/32.png) [@Oleg\_T](https://www.ruby-forum.com/u/Oleg_T)\
**Post date:** [July 23, 2008, 10:36am UTC](https://www.ruby-forum.com/t/code-review-yamlfixes/143853/1 "2008-07-23T10:36:48Z")

</div>

tfpt review “/shelveset:YamlFixes;REDMOND\olegtk”  
Comment :  
Removes obsolete YamlTest project from the solution.  
Fixes DateTime serialization and loading of timestamps.  
Fixes nil serialization.  
Implements YAML::add\_domain\_type() - user controlled parsing of domain  
types.  
Fixes loading binary data in Ruby context.  
Implements loading dates - instantiates Date class (from date.rb  
curently) instead of DateTime.  
Adds “require ‘date’” into yaml.rb to be compatible with MRI (it makes  
loading yaml much slower currently - need to implement Date class).  
Fixes bug with serializing a hash with a nil as a key.  
Implements loading abbreviated types (looks like obsoleted yaml 1.0  
only feature, but MRI tests it.  
[YAML Ain't Markup Language (YAML) (tm) 1.0](http://yaml.org/spec/history/2002-10-31.html#preview-family)).  
Splits SafeConstructor.ConstructYamlTimestamp into two methods so  
RubyConstructor can override Date construction.  
Fixes Time.at() - the argument must be a double type.  
Fixes Time.local() and Time.utc() - usec argument is actually  
microseconds, not milliseconds.

---

<div class="post-metadata">

**Author:** ![Oleg\_T](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/o/f19dbf/32.png) [@Oleg\_T](https://www.ruby-forum.com/u/Oleg_T)\
**Post date:** [July 23, 2008, 6:13pm UTC](https://www.ruby-forum.com/t/code-review-yamlfixes/143853/2 "2008-07-23T18:13:17Z")

</div>

Microseconds are being truncated to milliseconds instead being rounded.  
The least painful way to fix this is to .AddMilliseconds(1) to the  
result if usec \>= 500. (I might be okay with the truncation if it’s  
acknowledged in a comment.)

There’s a typo in a comment added to BaseConstructor.cs.

I’m not qualified to comment on the more YAML-specific stuff.

---

<div class="post-metadata">

**Author:** ![Oleg\_T](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/o/f19dbf/32.png) [@Oleg\_T](https://www.ruby-forum.com/u/Oleg_T)\
**Post date:** [July 23, 2008, 6:31pm UTC](https://www.ruby-forum.com/t/code-review-yamlfixes/143853/3 "2008-07-23T18:31:28Z")

</div>

1. add\_domain\_type should probably be split into 2 overloads - one for  
Regex and other for object converted to a string since the  
implementation is different.

2. Do not use dynamic sites for block invocation (RubyConstructor.cs):

return \_Block.Invoke(ctor.GetContext(), \_block,  
MutableString.Create(tag), ctor.ConstructPrimitive(node));

Use \_Block.Yield.

1. Representer.cs: why do we compare to two different “Null” objects?

Other than that looks good.

Tomas

---

<div class="post-metadata">

**Author:** ![Oleg\_T](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/o/f19dbf/32.png) [@Oleg\_T](https://www.ruby-forum.com/u/Oleg_T)\
**Post date:** [July 24, 2008, 8:49am UTC](https://www.ruby-forum.com/t/code-review-yamlfixes/143853/4 "2008-07-24T08:49:45Z")

</div>

Well, actually we don’t need to truncate or round altogether as DateTime  
has 0.1 microsecond precision. Here is a better way:

return new DateTime(year, month, day, hour, minute, second,  
DateTimeKind.Utc).AddTicks(microsecond \* 10);

If that looks ok, I’m going to submit the shelveset.

–  
Oleg

---

<div class="post-metadata">

**Author:** ![Oleg\_T](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/o/f19dbf/32.png) [@Oleg\_T](https://www.ruby-forum.com/u/Oleg_T)\
**Post date:** [July 24, 2008, 12:53am UTC](https://www.ruby-forum.com/t/code-review-yamlfixes/143853/5 "2008-07-24T00:53:41Z")

</div>

Thanks for comments, guys!  
3) some old rudimentary stuff, will clean it out.

–  
Oleg

---

<div class="post-metadata">

**Author:** ![Oleg\_T](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/o/f19dbf/32.png) [@Oleg\_T](https://www.ruby-forum.com/u/Oleg_T)\
**Post date:** [July 24, 2008, 8:56am UTC](https://www.ruby-forum.com/t/code-review-yamlfixes/143853/6 "2008-07-24T08:56:12Z")

</div>

Ah, right. Looks good. AddTicks is a reasonably cheap operations so  
there’s probably no compelling reason to special-case (microsecond ==  
0).