[sword-devel] [PATCH] Unused code

Dmitrijs Ledkovs dmitrij.ledkov at ubuntu.com
Thu Mar 22 16:22:31 MST 2012


Troy,

thank you for taking the time to look at them again.

On 22 March 2012 23:07, Troy A. Griffitts <scribe at crosswire.org> wrote:
> Dmitrijs,
>
> Thank you for the patches.  I will review your patches again, but this one
> specifically I would comment that I'm not in favor of removing commented out
> code just to make distro checks pass.  There is a reason the code is still
> there, though commented out.  I didn't write the URL class, but would
> venture to thing that the intent was to eventually support '#' mark, and
> code has been started but not completed.
>

- reduces readability of the code
- increases complexity
- why comment stuff out if there is version control system for this?
to e.g. resurrect

> Same with the isCommentary mark in osis2mod.  I would guess that the
> developers have plans to use that eventually to do special processing if the
> text is a commentary text rather than a Bible text.
>

ok. How about stripping this code and replace it with:

    #TODO add special processing for features X, Y, Z if text is commentary

that would be more useful and might prompt someone to write some code.

Dead, unused code looks untidy.

> I'm excited about the other patches (besides the checking return values
> patch which we disagreed on quite a number of months ago and I'll have to
> dig up my reasons then, but rather than a theoretical reason, I believe the
> patch at the time actually change the logic, which I was not happy with the
> new results).

Can you please remind me? I do not have all the archives of sword-devel.

Maybe we should wrap reads & write into a reusable functions which
does sanity checks and provide useful error messages back to the
application?!

I thought it's good practise to check for errors on IO code. Is there
any reason to not check for it? How would you want to check for it?

> And again, I believe these are all to make a pedantic distro
> code checker happy, yes?
>

Nothing to do with distro checks. Simply general code quality &
pragmatic codebase maintenance.


-- 
With regards,

Dmitrijs Ledkovs



More information about the sword-devel mailing list