# Code Review: StringGsubVersion

**URL:** <https://www.ruby-forum.com/t/code-review-stringgsubversion/142755>\
**Category:** IronRuby\
**Created:** [July 11, 2008, 3:22am UTC](https://www.ruby-forum.com/t/code-review-stringgsubversion/142755 "2008-07-11T03:22:39Z")\
**Posts on this page:** 4\
**Page:** 1

<div class="post-metadata">

**Author:** ![Dave\_Remy](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/9fc29f/32.png) [@Dave\_Remy](https://www.ruby-forum.com/u/Dave_Remy)\
**Post date:** [July 11, 2008, 3:22am UTC](https://www.ruby-forum.com/t/code-review-stringgsubversion/142755/1 "2008-07-11T03:22:39Z")

</div>

tfpt review “/shelveset:StringGsubVersion;REDMOND\dremy”  
Comment :  
This change fixes all of the gsub, each, and each\_line specs. Added  
aditional tests to gsub spec. The excluded gsub tests are only failing  
on unimplemented string methods (lstrip!, rstrip!) Implemented simple  
mutablestring version that is used to assure that mutation hasn’t  
occurred during an iteration.

tring#gsub with pattern and block sets $~ for access from the block  
String#gsub with pattern and block restores $~ after leaving the block  
String#gsub with pattern and block sets $~ to MatchData of last match  
and nil when there’s none for access from outside  
String#gsub with pattern and block raises a RuntimeError if the string  
is modified while substituting

This is a revision on a previous shelfset I submitted for these changes.  
Based on feedback I did a few things differently. I didn’t do anything  
special with version it is just a simple property implementation at this  
point. Shri’s points about potential multi-threaded issues and overflow  
are well taken but it sounds like we will pick this up as requirements  
dictate. I did revise the way version gets incremented. Rather than  
creating a single function that both requires frozen and increments  
(didn’t someone say sometime that function should only do one thing ;))  
I think this was a good thing, I ended up putting the Version++ in the  
logical place close to where a mutation actually occurs. This combined  
with putting tests for each mutation method caught some scenarios that  
would have failed.

---

<div class="post-metadata">

**Author:** ![Dave\_Remy](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/9fc29f/32.png) [@Dave\_Remy](https://www.ruby-forum.com/u/Dave_Remy)\
**Post date:** [July 11, 2008, 3:47am UTC](https://www.ruby-forum.com/t/code-review-stringgsubversion/142755/2 "2008-07-11T03:47:16Z")

</div>

I think it would be better to increment the version inside MutableString  
class (not in -Ops). I can do that transition when I’ll revisit  
frozen/tainted flags on MutableString. So go ahead with your change.

Tomas

---

<div class="post-metadata">

**Author:** ![Dave\_Remy](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/9fc29f/32.png) [@Dave\_Remy](https://www.ruby-forum.com/u/Dave_Remy)\
**Post date:** [July 11, 2008, 3:58am UTC](https://www.ruby-forum.com/t/code-review-stringgsubversion/142755/3 "2008-07-11T03:58:22Z")

</div>

Ok, cool. Sounds good.

Are you thinking of a method in MutableString like IncrementVersion()?  
Or something?

---

<div class="post-metadata">

**Author:** ![Dave\_Remy](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/d/9fc29f/32.png) [@Dave\_Remy](https://www.ruby-forum.com/u/Dave_Remy)\
**Post date:** [July 11, 2008, 4:01am UTC](https://www.ruby-forum.com/t/code-review-stringgsubversion/142755/4 "2008-07-11T04:01:53Z")

</div>

The version updates would be internal detail of MutableString. You would  
be able to ask for the current version from other classes, but not  
change it. The operations on MutableString will do.

Tomas