Skip to content

Conversation

@Erasiloco
Copy link
Contributor

@Erasiloco Erasiloco self-assigned this Jul 11, 2020
@Erasiloco Erasiloco requested a review from bzaar July 11, 2020 18:01
Copy link
Member

@bzaar bzaar left a comment

Choose a reason for hiding this comment

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

В остальном все нормально, но пока не будем мерджить, пока не будет принят основной PR.


using (var client = _newClient())
{
client.AddParam("n", n.ToString());
Copy link
Member

Choose a reason for hiding this comment

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

Я бы добавил: ToString(CultureInfo.InvariantCulture). Иначе все может работать, пока на хостинге добрый дядя не поменяет региональные настройки.

}
}

public SummaPropisResult SpellSum(decimal n, string currency,
Copy link
Member

Choose a reason for hiding this comment

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

Переименуй SpellSum в GetPropis, пожалуйста.

Copy link
Member

Choose a reason for hiding this comment

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

А SummaPropisResult - в Propis.

@Erasiloco
Copy link
Contributor Author

@bzaar Не подскажете, почему не может создать ответ из Propis-а? Разные варианты атрибутов попробовал поставить внутри класса, но все равно не может сформировать.

@bzaar
Copy link
Member

bzaar commented Jul 29, 2020

Не очень понял. Что именно ты делаешь и что получаешь?

@Erasiloco
Copy link
Contributor Author

Не очень понял. Что именно ты делаешь и что получаешь?

Не в том PR-e написал вопрос.

@Erasiloco Erasiloco requested a review from bzaar August 2, 2020 04:54
Copy link
Member

@bzaar bzaar left a comment

Choose a reason for hiding this comment

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

См. комментарии

client.AddParam("nbsp", nbsp);
client.AddParam("delim", delim);

return client.GetObject<Propis>("/russian/propis");
Copy link
Member

Choose a reason for hiding this comment

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

А обработка ошибок? Что если сервис вернет ошибку 400? Нужно преобразовать ее в известное исключение.

public class Propis
{
[DataMember]
public string propis1 { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

[XmlElement]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

А зачем он? Вроде в других классах такого не было.

public string propis1 { get; set; }

[DataMember]
public string propis2 { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

В C# свойства принято называть с большой буквы: Propis2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ок

@Erasiloco Erasiloco requested a review from bzaar August 20, 2020 16:01
@Erasiloco
Copy link
Contributor Author

Добавил изменения по замечаниям.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants