-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix and improve duplicates #240
Conversation
dup += 1 | ||
|
||
if dup == 13 | ||
value_dups.push("Faturas: #{det.css('nDup').text} - #{format_dup_date(det, det.css('dVenc').text)} - R$ #{BrDanfe::Helper.numerify(det.css('vDup').text.to_f)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
if dup == 13 | ||
value_dups.push("Faturas: #{det.css('nDup').text} - #{format_dup_date(det, det.css('dVenc').text)} - R$ #{BrDanfe::Helper.numerify(det.css('vDup').text.to_f)}") | ||
elsif dup > 13 | ||
value_dups.push("#{det.css('nDup').text} - #{format_dup_date(det, det.css('dVenc').text)} - R$ #{BrDanfe::Helper.numerify(det.css('vDup').text.to_f)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar blocks of code found in 2 locations. Consider refactoring.
@@ -388,6 +479,78 @@ | |||
<pesoB>3300.000</pesoB> | |||
</vol> | |||
</transp> | |||
<cobr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrigir indentação.
|
||
render_titles_and_box(x, y) | ||
det_count = 0 | ||
|
||
@xml.collect('xmlns', 'dup') do |det| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recomendo filtrar a coleção, usando por exemplo, filtro por range.
Inclusive, remover o uso da variável det_count
para usar o index
do próprio loop.
Examplo:
@xml.collect('xmlns', 'dup')[..12].each_with_index do |det|
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defina uma constante: DUP_MAX_QUANTITY = 12.
Devido ao comportamento do método collect
a sugestão acima precisa ser ajustada para:
@xml.collect('xmlns', 'dup') { _1 }[..DUP_MAX_QUANTITY - 1].each do |det|
end
Use no outro local que faz esse loop também.
dup = 0 | ||
value_dups = [] | ||
|
||
@xml.collect('xmlns', 'dup') do |det| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recomendo filtrar a coleção, usando por exemplo, filtro por range.
Inclusive, remover o uso da variável dup
para usar o index
do próprio loop.
Examplo:
@xml.collect('xmlns', 'dup')[13..].each_with_index do |det|
end
dup += 1 | ||
value = "#{det.css('nDup').text} - #{format_dup_date(det, det.css('dVenc').text)} - R$ #{BrDanfe::Helper.numerify(det.css('vDup').text.to_f)}" | ||
|
||
if dup == 13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a constante definida
if dup == 13 | |
if dup == BrDanfe::DanfeLib::NfeLib::Dup::DUP_MAX_QUANTITY |
|
||
render_titles_and_box(x, y) | ||
det_count = 0 | ||
|
||
@xml.collect('xmlns', 'dup') do |det| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Defina uma constante: DUP_MAX_QUANTITY = 12.
Devido ao comportamento do método collect
a sugestão acima precisa ser ajustada para:
@xml.collect('xmlns', 'dup') { _1 }[..DUP_MAX_QUANTITY - 1].each do |det|
end
Use no outro local que faz esse loop também.
Code Climate has analyzed commit fd439b5 and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (99% is the threshold). This pull request will bring the total coverage in the repository to 99.5% (0.0% change). View more on Code Climate. |
No description provided.