-
Notifications
You must be signed in to change notification settings - Fork 22
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
Constants following PascalCase convention #71
base: develop
Are you sure you want to change the base?
Changes from 1 commit
d50b8b8
46214e0
8fd7136
3fbb0e1
9b16490
17d446c
f405de3
4a90c80
c43eb8c
d53ea29
d453229
2da16bf
d66b682
2279dc8
5f8e799
c153018
1aa819d
9962a6d
f10e0d3
50a81c3
4240c77
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,10 +17,10 @@ public class PaymentChatTest : BunqSdkTestBase | |
/// <summary> | ||
/// Config values. | ||
/// </summary> | ||
private const string Amount = "0.01"; | ||
private const string Currency = "EUR"; | ||
private const string Description = "Payment From C# Test"; | ||
private const string Text = "test msg send from C# test"; | ||
private const string PaymentChatAmountEur = "0.01"; | ||
private const string PaymentChatCurrency = "EUR"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please suffix the currency |
||
private const string PaymentChatDescription = "Payment From C# Test"; | ||
private const string PaymentChatText = "test msg send from C# test"; | ||
|
||
private static readonly int UserId = Config.GetUserId(); | ||
private static readonly int MonetaryAccountId = Config.GetMonetarytAccountId(); | ||
|
@@ -43,7 +43,7 @@ public void TestSendPaymentChat() | |
|
||
var chatMessageMap = new Dictionary<string, object> | ||
{ | ||
{ChatMessageText.FIELD_TEXT, Text} | ||
{ChatMessageText.FIELD_TEXT, PaymentChatText} | ||
}; | ||
ChatMessageText.Create(ApiContext, chatMessageMap, UserId, chatId); | ||
} | ||
|
@@ -52,9 +52,9 @@ private static int CreatePaymentAndGetId() | |
{ | ||
var requestMap = new Dictionary<string, object> | ||
{ | ||
{Payment.FIELD_AMOUNT, new Amount(Amount, Currency)}, | ||
{Payment.FIELD_AMOUNT, new Amount(PaymentChatAmountEur, PaymentChatCurrency)}, | ||
{Payment.FIELD_COUNTERPARTY_ALIAS, CounterPartyAlias}, | ||
{Payment.FIELD_DESCRIPTION, Description}, | ||
{Payment.FIELD_DESCRIPTION, PaymentChatDescription}, | ||
}; | ||
|
||
return Payment.Create(ApiContext, requestMap, UserId, MonetaryAccountId).Value; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,10 @@ public class RequestInquiryTest : BunqSdkTestBase | |
/// <summary> | ||
/// Config values. | ||
/// </summary> | ||
private const string Amount = "0.01"; | ||
private const string Currency = "EUR"; | ||
private const string Description = "C# test Payment"; | ||
private const string Status = "ACCEPTED"; | ||
private const string RequestInquiryAmountEur = "0.01"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd change naming to something like ValueAmountEur (indicating that it is a value to be used in the test). RequestInquiryStatus looks a lot like the actual status objects now imo (@OGKevin 👀 ) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sandervdo Argh you have a point 😭 stupid naming convention. @AnTao lets group these values by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will change this group with the keyword 'Value' then. 👍 @OGKevin @sandervdo is there a coding style guide available? That would be 👌 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm No there is no coding style publicly available. ATM we have a mixture of the Microsoft Conde stye and or own. E.g. the constants grouping and no plurals. Might be a good idea to write a wiki about the code style 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, last question. Shall we also change the constants to contain 'Value' grouping in the test sample classes? Thanks guys! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, lets be consistent 👍 And thank you 🤗 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, last remark, Imho I believe we should use Value group in AmountEur or CurrencyEur in the case of this constants. And also group with value Field for others used in Field constants. What do you think ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All A field typically looks like this: public const string FieldStatus = "status"; Instead of the actual value that the status field will hold, in this case |
||
private const string RequestInquiryCurrency = "EUR"; | ||
private const string RequestInquiryDescription = "C# test Payment"; | ||
private const string RequestInquiryStatus = "ACCEPTED"; | ||
private const int IndexFirst = 0; | ||
|
||
private static readonly int UserId = Config.GetUserId(); | ||
|
@@ -30,7 +30,7 @@ public class RequestInquiryTest : BunqSdkTestBase | |
/// <summary> | ||
/// API context to use for the test API calls. | ||
/// </summary> | ||
private static readonly ApiContext API_CONTEXT = GetApiContext(); | ||
private static readonly ApiContext ApiContext = GetApiContext(); | ||
|
||
/// <summary> | ||
/// Tests sending a request from monetary account 1 to monetary account 2 and accepting this request. | ||
|
@@ -40,28 +40,27 @@ public void TestRequestInquiry() | |
{ | ||
var requestMap = new Dictionary<string, object> | ||
{ | ||
{RequestInquiry.FIELD_AMOUNT_INQUIRED, new Amount(Amount, Currency)}, | ||
{RequestInquiry.FIELD_AMOUNT_INQUIRED, new Amount(RequestInquiryAmountEur, RequestInquiryCurrency)}, | ||
{RequestInquiry.FIELD_COUNTERPARTY_ALIAS, CounterSelfParty}, | ||
{RequestInquiry.FIELD_DESCRIPTION, Description}, | ||
{RequestInquiry.FIELD_DESCRIPTION, RequestInquiryDescription}, | ||
{RequestInquiry.FIELD_ALLOW_BUNQME, false} | ||
}; | ||
|
||
RequestInquiry.Create(API_CONTEXT, requestMap, UserId, MonetaryAccountId); | ||
RequestInquiry.Create(ApiContext, requestMap, UserId, MonetaryAccountId); | ||
|
||
Assert.Equal(Status, AcceptRequest()); | ||
Assert.Equal(RequestInquiryStatus, AcceptRequest()); | ||
} | ||
|
||
private static string AcceptRequest() | ||
{ | ||
var requestResponseId = RequestResponse | ||
.List(API_CONTEXT, UserId, SecondMonetaryAccountId).Value[IndexFirst].Id.Value; | ||
var requestResponseId = RequestResponse.List(ApiContext, UserId, SecondMonetaryAccountId).Value[IndexFirst].Id.Value; | ||
|
||
var requestMap = new Dictionary<string, object> | ||
{ | ||
{RequestResponse.FIELD_STATUS, Status} | ||
{RequestResponse.FIELD_STATUS, RequestInquiryStatus} | ||
}; | ||
|
||
return RequestResponse.Update(API_CONTEXT, requestMap, UserId, SecondMonetaryAccountId, | ||
return RequestResponse.Update(ApiContext, requestMap, UserId, SecondMonetaryAccountId, | ||
requestResponseId).Value.Status; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,12 +16,12 @@ public class TabUsageSingleTest : BunqSdkTestBase | |
/// <summary> | ||
/// Config values | ||
/// </summary> | ||
private const string TabDescription = "Pay the tab for Java test please."; | ||
private const string StatusOpen = "OPEN"; | ||
private const string Amount = "42.00"; | ||
private const string Currency = "EUR"; | ||
private const string TabItemDescription = "Super expensive java tea"; | ||
private const string StatusWaiting = "WAITING_FOR_PAYMENT"; | ||
private const string TabUsageSingleDescription = "Pay the tab for Java test please."; | ||
private const string TabUsageSingleStatusOpen = "OPEN"; | ||
private const string TabAmountEur = "42.00"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please be consistent 😝 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The constant is also used along with the TabItemShop entity. Changing it for consistency! |
||
private const string TabCurrency = "EUR"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please suffix |
||
private const string TabItemShopDescription = "Super expensive java tea"; | ||
private const string TabUsageSingleStatusWaiting = "WAITING_FOR_PAYMENT"; | ||
|
||
private static readonly int UserId = Config.GetUserId(); | ||
private static readonly int MonetaryAccountId = Config.GetMonetarytAccountId(); | ||
|
@@ -46,7 +46,7 @@ public void TestCreateTabAndUpdate() | |
|
||
var updateTabMap = new Dictionary<string, object> | ||
{ | ||
{TabUsageSingle.FIELD_STATUS, StatusWaiting} | ||
{TabUsageSingle.FIELD_STATUS, TabUsageSingleStatusWaiting} | ||
}; | ||
TabUsageSingle.Update(ApiContext, updateTabMap, UserId, MonetaryAccountId, CashRegisterId, tabUuid); | ||
|
||
|
@@ -62,9 +62,9 @@ private static string CreateTabAndGetUuid() | |
{ | ||
var createTabMap = new Dictionary<string, object> | ||
{ | ||
{TabUsageSingle.FIELD_DESCRIPTION, TabDescription}, | ||
{TabUsageSingle.FIELD_STATUS, StatusOpen}, | ||
{TabUsageSingle.FIELD_AMOUNT_TOTAL, new Amount(Amount, Currency)} | ||
{TabUsageSingle.FIELD_DESCRIPTION, TabUsageSingleDescription}, | ||
{TabUsageSingle.FIELD_STATUS, TabUsageSingleStatusOpen}, | ||
{TabUsageSingle.FIELD_AMOUNT_TOTAL, new Amount(TabAmountEur, TabCurrency)} | ||
}; | ||
|
||
return TabUsageSingle.Create(ApiContext, createTabMap, UserId, MonetaryAccountId, | ||
|
@@ -75,8 +75,8 @@ private static void AddTabItem(string tabUuid) | |
{ | ||
var tabItemMap = new Dictionary<string, object> | ||
{ | ||
{TabItemShop.FIELD_AMOUNT, new Amount(Amount, Currency)}, | ||
{TabItemShop.FIELD_DESCRIPTION, TabItemDescription} | ||
{TabItemShop.FIELD_AMOUNT, new Amount(TabAmountEur, TabCurrency)}, | ||
{TabItemShop.FIELD_DESCRIPTION, TabItemShopDescription} | ||
}; | ||
TabItemShop.Create(ApiContext, tabItemMap, UserId, MonetaryAccountId, CashRegisterId, tabUuid); | ||
} | ||
|
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.
Lets make this name future proof,
MonetaryAccountBankCurrencyEur
. If we start supporting other currencies then there will be less names to change 👍