| Subject: | Unreachable code in Dump() |
sub Dump() currently (Sat Nov 02 2013) contains code not exercised by the YAML-Tiny test suite (see: http://thenceforward.net/YAML-Tiny/coverage/lib-YAML-Tiny-pm.html starting at line 667). This could have one of two explanations: (a) we haven't gotten around to writing a test for the "not defined $string" case; or (b) the string can never be undefined, hence the code is unreachable and excisable.
My hunch is that it's (b). We need to ask: Can the return value of "YAML::Tiny->new(@_)->write_string" ever be undefined?
Suppose we refactor write_string() a bit to make its return values more self-evident:
diff --git a/lib/YAML/Tiny.pm b/lib/YAML/Tiny.pm
index b92286f..8f94492 100644
--- a/lib/YAML/Tiny.pm
+++ b/lib/YAML/Tiny.pm
@@ -527,11 +527,13 @@ sub write_string {
};
if ( ref $@ eq 'SCALAR' ) {
return $self->_error(${$@});
- } elsif ( $@ ) {
- $self->_error($@);
}
-
- join '', map { "$_\n" } @lines;
+ else {
+ if ( $@ ) {
+ $self->_error($@);
+ }
+ return join '', map { "$_\n" } @lines;
+ }
}
# use XXX -with => 'YAML::XS';
We now see clearly all the points where write_string() will return.
(1) return '' unless ref $self && @$self;
That's returning a defined, though false, value.
(2) return $self->_error(${$@});
That returns a reference, which I doubt can be undefined. In any event, that statement is already being exercised by the test suite.
(3) return join '', map { "$_\n" } @lines;
That's going to be a defined, though perhaps empty, string.
So I infer that write_string() cannot return an undefined value. If so, then the check for definedness inside Dump() is superfluous and can be removed.
Do you agree?
Thank you very much.
Jim Keenan
| Subject: | write_string.diff |
diff --git a/lib/YAML/Tiny.pm b/lib/YAML/Tiny.pm
index b92286f..8f94492 100644
--- a/lib/YAML/Tiny.pm
+++ b/lib/YAML/Tiny.pm
@@ -527,11 +527,13 @@ sub write_string {
};
if ( ref $@ eq 'SCALAR' ) {
return $self->_error(${$@});
- } elsif ( $@ ) {
- $self->_error($@);
}
-
- join '', map { "$_\n" } @lines;
+ else {
+ if ( $@ ) {
+ $self->_error($@);
+ }
+ return join '', map { "$_\n" } @lines;
+ }
}
# use XXX -with => 'YAML::XS';