#16935: Faster palindromes function for the Words library
-------------------------------------+-------------------------------------
Reporter: nadialafreniere | Owner:
Type: enhancement | Status: needs_review
Priority: major | Milestone: sage-6.4
Component: combinatorics | Resolution:
Keywords: words, finite | Merged in:
words, palindromes | Reviewers:
Authors: Nadia Lafrenière | Work issues:
Report Upstream: N/A | Commit:
Branch: | def64a1a59757069be2840256a41dbc5951d5e07
u/nadialafreniere/16935 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by slabbe):
It is difficult for me to take a look at the code itself when the syntax
convention are not followed.
1. One empty line is nice to separate block of code. Two consecutive empty
lines should be avoided.
2. Never put an empty line after a `for` statement.
3. Try to keep comment on one line. Rephrase if necessary.
4. One line should be less than 80 characters.
5. If a comment takes more than one line, then avoid any indentation for
the second line.
6. A comment should always start at the actual indentation level.
7. Why do you write {{{k = Integer(0) }}} instead of just {{{k=0}}}?
8. {{{long_lpc}}} should not appear anymore (code + doctest).
9. Slices of words should not be changed into tuple to make the equality
test. The code in that class is mathematical. It should not play with the
internal representation. Suppose your word is already represented by a
tuple. Why change it into a tuple?
10. Remove useless comment like
{{{
# Return the length of the palindrome
return p
}}}
11. Add comments where it is necessary:
{{{
except IndexError:
pass
}}}
Why such an error is not an error? It seems strange to me to choose such
an implementation...
--
Ticket URL: <http://trac.sagemath.org/ticket/16935#comment:10>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
