Re: [patch] Remove unused code in SVN::logReceiver()

Stefan wrote:

> Daniel Rall wrote:
> > [[[
> > Remove used code.
> ^^^^
>
> ??? :-)

Sorry! What is it with me and you and inverted negatives in my
emails? :-P

[[[
Remove unused code.

* src/SVN/SVN.cpp
  (SVN::logReceiver): Remove unnecessary "if" statement around log
   notification, and redundant "return error" statement.
]]]

...
> > Index: src/SVN/SVN.cpp
> > ===================================================================
> > --- src/SVN/SVN.cpp (revision 8244)
> > +++ src/SVN/SVN.cpp (working copy)
> > @@ -1214,10 +1214,7 @@
> > SVN_ERR (svn->cancel(baton));
> > #pragma warning(pop)
> >
> > - if (svn->Log(rev, author_native, date_native, msg_native, arChangedPaths, time_temp, filechanges, copies, actions))
> > - {
> > - return error;
> > - }
> > + svn->Log(rev, author_native, date_native, msg_native, arChangedPaths, time_temp, filechanges, copies, actions);
> > return error;
>
> We can't remove that code: the Log() method is virtual, and other
> classes have to override it to actually get the log data. Sure, right
> now no other class/dialog returns an error, but I like to have the
> option to maybe do that later (e.g. out-of-memory errors, other errors
> which might occur while trying to use the log data, ...)

Hmm, I'm not following. The SVN::Log() API returns a BOOL. The
"error" variable you see above is local to the SVN::logReceiver()
method. Regardless of whether SVN::Log() returns true or false, we
always return "error". Because of this, the "if" block which inspects
the return value of SVN::logReceiver() is unnecessary.

Here's two condensed forms of the method for comparison:

logReceiver()
{
  svn_error_t *error = NULL;
  ...
  if (svn->Log(...))
  {
    return error;
  }
  return error;
}

logReceiver()
{
  svn_error_t *error = NULL;
  ...
  svn->Log(...);
  return error;
}

Both forms to the same thing (call Log() and return "error"), but the
second form does it more clearly and concisely.

- Dan