Skip to content
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

iss29 #104

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

iss29 #104

wants to merge 12 commits into from

Conversation

uom42
Copy link
Collaborator

@uom42 uom42 commented Jan 13, 2022

No description provided.

@navferty navferty self-requested a review January 13, 2022 16:29
Copy link
Owner

@navferty navferty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Предлагаю переделать алгоритм, чтобы присвоение распарсенных значений ячеек происходило за один вызов батчем, а также добавить тесты с книгой (лист с исходными значениями, второй лист с целевыми значениями и форматами, которые должны быть - по тем же адресам ячеек).

NavfertyExcelAddIn/ParseNumerics/DecimalParser.cs Outdated Show resolved Hide resolved
const string CURRENCY_TEMPLATE = @"{CUR}";

bool autoCalcEnabled = false;
try { autoCalcEnabled = selection.Worksheet.EnableCalculation; } catch { }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

думаю что try-catch тут излишни

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Падает на set .EnableCalculation иногда непредсказуемо. Причину пока не знаю, то работает, то нет.

try
{

selection.ApplyForEachCellOfType2<string, object>(
Copy link
Owner

@navferty navferty Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Как обсуждали, предлагаю отказаться от ApplyForEachCellOfType2, вручную получив по каждой area массив исходных значений (object[,]), соответствующую коллекцию NumericParseResult. Преобразовать коллекцию в двумерных массив (object[,]), присвоить значения одним вызовом, после чего пробежаться по коллекции NumericParseResult, чтобы задать формат ячейкам, для которых у результата парсинга есть значок валюты

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

работаю над этим

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

надо тесты допилить с Cells, и проверить, а то вообще все тесты развалятся тогда...


var cells = new Mock<Range>(MockBehavior.Default);
cells.Setup(x => x.GetEnumerator()).Returns(new[] { selection.Object }.GetEnumerator());
//selection.SetupSet(x => x.get_Cells = It.Is<object[,]>(z => VerifyParsed(z)));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

просьба удалить неиспользуемый закомментированный код

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тесты недоделаны, немогу Range.Cells нормально в тесте сделать... тест не проходит....

@uom42 uom42 added the bug Something isn't working label Jan 14, 2022
@uom42 uom42 linked an issue Jan 14, 2022 that may be closed by this pull request
@uom42 uom42 self-assigned this Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parse numerics - preserve currency
2 participants