Synchronized implementation of a bank account in JavaUsing volatile instead of synchronized for a simulationSynchronized block over concurrent collectionsOOP bank databaseThread safety/Transaction enforcerImplementation of stackSimple bank accountSynchronized Queue Wrapper C++11Singleton with a volatile and synchronized instanceBasic Java bank accountSimulate BankAccount in Java
A Ri-diddley-iley Riddle
Why is there so much iron?
Deletion of copy-ctor & copy-assignment - public, private or protected?
Can a medieval gyroplane be built?
Does the attack bonus from a Masterwork weapon stack with the attack bonus from Masterwork ammunition?
What are substitutions for coconut in curry?
PTIJ: Do Irish Jews have "the luck of the Irish"?
What does "Four-F." mean?
Pronounciation of the combination "st" in spanish accents
Light propagating through a sound wave
Worshiping one God at a time?
How to get the n-th line after a grepped one?
Suggestions on how to spend Shaabath (constructively) alone
Synchronized implementation of a bank account in Java
PTIJ What is the inyan of the Konami code in Uncle Moishy's song?
PTIJ: Why do we blow Shofar on Rosh Hashana and use a Lulav on Sukkos?
Relation between independence and correlation of uniform random variables
What does Jesus mean regarding "Raca," and "you fool?" - is he contrasting them?
Could Sinn Fein swing any Brexit vote in Parliament?
Hausdorff dimension of the boundary of fibres of Lipschitz maps
Turning a hard to access nut?
Is it insecure to send a password in a `curl` command?
Using Past-Perfect interchangeably with the Past Continuous
Recruiter wants very extensive technical details about all of my previous work
Synchronized implementation of a bank account in Java
Using volatile instead of synchronized for a simulationSynchronized block over concurrent collectionsOOP bank databaseThread safety/Transaction enforcerImplementation of stackSimple bank accountSynchronized Queue Wrapper C++11Singleton with a volatile and synchronized instanceBasic Java bank accountSimulate BankAccount in Java
$begingroup$
I am trying to implement a bank account with Java in a thread safe way. My code looks like:
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
/**
* Class to represent an account, it also provides with methods to add and withdraw amount from the account.
*
* @author Karan Khanna
* @version 1.0
* @since 3/17/2019
*/
public class Account
private ReadWriteLock accountLock;
private BigDecimal balance;
private String accountNumber;
private String accountHolder;
public Account(String accountNumber, String accountHolder)
this.balance = new BigDecimal(0);
this.accountNumber = accountNumber;
this.accountHolder = accountHolder;
this.accountLock = new ReentrantReadWriteLock();
public double getBalance()
this.accountLock.readLock().lock();
double balance = this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
this.accountLock.readLock().unlock();
return balance;
public String getAccountNumber()
return accountNumber;
public String getAccountHolder()
return accountHolder;
public ReadWriteLock getAccountLock()
return accountLock;
public void addAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.add(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
public void withdrawAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.subtract(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
I am looking for feedback for the implementation.
java multithreading thread-safety
$endgroup$
add a comment |
$begingroup$
I am trying to implement a bank account with Java in a thread safe way. My code looks like:
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
/**
* Class to represent an account, it also provides with methods to add and withdraw amount from the account.
*
* @author Karan Khanna
* @version 1.0
* @since 3/17/2019
*/
public class Account
private ReadWriteLock accountLock;
private BigDecimal balance;
private String accountNumber;
private String accountHolder;
public Account(String accountNumber, String accountHolder)
this.balance = new BigDecimal(0);
this.accountNumber = accountNumber;
this.accountHolder = accountHolder;
this.accountLock = new ReentrantReadWriteLock();
public double getBalance()
this.accountLock.readLock().lock();
double balance = this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
this.accountLock.readLock().unlock();
return balance;
public String getAccountNumber()
return accountNumber;
public String getAccountHolder()
return accountHolder;
public ReadWriteLock getAccountLock()
return accountLock;
public void addAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.add(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
public void withdrawAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.subtract(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
I am looking for feedback for the implementation.
java multithreading thread-safety
$endgroup$
add a comment |
$begingroup$
I am trying to implement a bank account with Java in a thread safe way. My code looks like:
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
/**
* Class to represent an account, it also provides with methods to add and withdraw amount from the account.
*
* @author Karan Khanna
* @version 1.0
* @since 3/17/2019
*/
public class Account
private ReadWriteLock accountLock;
private BigDecimal balance;
private String accountNumber;
private String accountHolder;
public Account(String accountNumber, String accountHolder)
this.balance = new BigDecimal(0);
this.accountNumber = accountNumber;
this.accountHolder = accountHolder;
this.accountLock = new ReentrantReadWriteLock();
public double getBalance()
this.accountLock.readLock().lock();
double balance = this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
this.accountLock.readLock().unlock();
return balance;
public String getAccountNumber()
return accountNumber;
public String getAccountHolder()
return accountHolder;
public ReadWriteLock getAccountLock()
return accountLock;
public void addAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.add(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
public void withdrawAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.subtract(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
I am looking for feedback for the implementation.
java multithreading thread-safety
$endgroup$
I am trying to implement a bank account with Java in a thread safe way. My code looks like:
import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
/**
* Class to represent an account, it also provides with methods to add and withdraw amount from the account.
*
* @author Karan Khanna
* @version 1.0
* @since 3/17/2019
*/
public class Account
private ReadWriteLock accountLock;
private BigDecimal balance;
private String accountNumber;
private String accountHolder;
public Account(String accountNumber, String accountHolder)
this.balance = new BigDecimal(0);
this.accountNumber = accountNumber;
this.accountHolder = accountHolder;
this.accountLock = new ReentrantReadWriteLock();
public double getBalance()
this.accountLock.readLock().lock();
double balance = this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
this.accountLock.readLock().unlock();
return balance;
public String getAccountNumber()
return accountNumber;
public String getAccountHolder()
return accountHolder;
public ReadWriteLock getAccountLock()
return accountLock;
public void addAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.add(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
public void withdrawAmount(double amount)
this.accountLock.writeLock().lock();
this.balance.subtract(new BigDecimal(amount));
this.accountLock.writeLock().unlock();
I am looking for feedback for the implementation.
java multithreading thread-safety
java multithreading thread-safety
asked 12 hours ago
Karan KhannaKaran Khanna
1856
1856
add a comment |
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
In terms of the basic thread locking, it looks like it is doing the right thing, but there are a number of issues in how you are calculating the account balance, and also some escaped locking as well.
Note, your post is titled "Synchronized implementation", but it is not, it is a locked implementation. Synchronization is different, and, in this case, it may be a simpler mechanism.
Locking
Even if you don't catch exceptions, you should always use the try/finally mechanism for locking. Here, for example, it's possible that the addition may throw an exception (even though you don't catch it):
public void addAmount(double amount)
this.accountLock.writeLock().lock();
try
this.balance.add(new BigDecimal(amount));
finally
this.accountLock.writeLock().unlock();
In case you think that's extreme, well, the amount could be NaN
or infinity
, and that would throw a NumberFormatException
, etc. Even if it were impossible for the logic to throw an error, you should still use the try/finally mechanism because it makes the logic obvious.
The balance method has the most to gain:
public double getBalance()
this.accountLock.readLock().lock();
try
return this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
finally
this.accountLock.readLock().unlock();
You are also leaking the lock through the public method to get it. You really should not allow other people to manipulate the lock strategy you have in your class. It is intended to be internal for a reason.
Speaking of that lock, you should also make it final...
private final ReadWriteLock accountLock;
Bugs
The most glaring issue is not with your locking, but with the balance management itself. BigDecimals are immutable. They cannot be changed. This does nothing: this.balance.add(new BigDecimal(amount));
.... that should be this.balance = this.balance.add(new BigDecimal(amount));
.
The accountNumber
and accountHolder
should be final as well.
Finally, the getBalance method will not always return a 2-decimal double value. Not all values in binary floating-point are representable in decimal.
$endgroup$
add a comment |
$begingroup$
double
is not a good choice to use for currency in Java. The better option is BigDecimal
(which you are using for the internal balance, but not for the parameters passed to the addAmount
and withdrawAmount
methods). A better approach would be to make those methods take a BigDecimal
parameter instead (and to use BigDecimal
everywhere in your code that deals with currency amounts).
If for some reason those methods need to take a double
parameter then you should not convert it to a BigDecimal
with the new BigDecimal(double)
constructor - this will give an inaccurate conversion and an unexpected (and incorrect) value for the balance after the add/withdraw operation. For example, the following test fails:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(new BigDecimal(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
with the error
java.lang.AssertionError:
Expected: is <0.1>
but: was <0.1000000000000000055511151231257827021181583404541015625>
The correct way to convert from a double
to a BigDecimal
is to use BigDecimal.valueOf(double)
. For example, changing the middle line in the above test will make it pass:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(BigDecimal.valueOf(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
New contributor
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215616%2fsynchronized-implementation-of-a-bank-account-in-java%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
In terms of the basic thread locking, it looks like it is doing the right thing, but there are a number of issues in how you are calculating the account balance, and also some escaped locking as well.
Note, your post is titled "Synchronized implementation", but it is not, it is a locked implementation. Synchronization is different, and, in this case, it may be a simpler mechanism.
Locking
Even if you don't catch exceptions, you should always use the try/finally mechanism for locking. Here, for example, it's possible that the addition may throw an exception (even though you don't catch it):
public void addAmount(double amount)
this.accountLock.writeLock().lock();
try
this.balance.add(new BigDecimal(amount));
finally
this.accountLock.writeLock().unlock();
In case you think that's extreme, well, the amount could be NaN
or infinity
, and that would throw a NumberFormatException
, etc. Even if it were impossible for the logic to throw an error, you should still use the try/finally mechanism because it makes the logic obvious.
The balance method has the most to gain:
public double getBalance()
this.accountLock.readLock().lock();
try
return this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
finally
this.accountLock.readLock().unlock();
You are also leaking the lock through the public method to get it. You really should not allow other people to manipulate the lock strategy you have in your class. It is intended to be internal for a reason.
Speaking of that lock, you should also make it final...
private final ReadWriteLock accountLock;
Bugs
The most glaring issue is not with your locking, but with the balance management itself. BigDecimals are immutable. They cannot be changed. This does nothing: this.balance.add(new BigDecimal(amount));
.... that should be this.balance = this.balance.add(new BigDecimal(amount));
.
The accountNumber
and accountHolder
should be final as well.
Finally, the getBalance method will not always return a 2-decimal double value. Not all values in binary floating-point are representable in decimal.
$endgroup$
add a comment |
$begingroup$
In terms of the basic thread locking, it looks like it is doing the right thing, but there are a number of issues in how you are calculating the account balance, and also some escaped locking as well.
Note, your post is titled "Synchronized implementation", but it is not, it is a locked implementation. Synchronization is different, and, in this case, it may be a simpler mechanism.
Locking
Even if you don't catch exceptions, you should always use the try/finally mechanism for locking. Here, for example, it's possible that the addition may throw an exception (even though you don't catch it):
public void addAmount(double amount)
this.accountLock.writeLock().lock();
try
this.balance.add(new BigDecimal(amount));
finally
this.accountLock.writeLock().unlock();
In case you think that's extreme, well, the amount could be NaN
or infinity
, and that would throw a NumberFormatException
, etc. Even if it were impossible for the logic to throw an error, you should still use the try/finally mechanism because it makes the logic obvious.
The balance method has the most to gain:
public double getBalance()
this.accountLock.readLock().lock();
try
return this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
finally
this.accountLock.readLock().unlock();
You are also leaking the lock through the public method to get it. You really should not allow other people to manipulate the lock strategy you have in your class. It is intended to be internal for a reason.
Speaking of that lock, you should also make it final...
private final ReadWriteLock accountLock;
Bugs
The most glaring issue is not with your locking, but with the balance management itself. BigDecimals are immutable. They cannot be changed. This does nothing: this.balance.add(new BigDecimal(amount));
.... that should be this.balance = this.balance.add(new BigDecimal(amount));
.
The accountNumber
and accountHolder
should be final as well.
Finally, the getBalance method will not always return a 2-decimal double value. Not all values in binary floating-point are representable in decimal.
$endgroup$
add a comment |
$begingroup$
In terms of the basic thread locking, it looks like it is doing the right thing, but there are a number of issues in how you are calculating the account balance, and also some escaped locking as well.
Note, your post is titled "Synchronized implementation", but it is not, it is a locked implementation. Synchronization is different, and, in this case, it may be a simpler mechanism.
Locking
Even if you don't catch exceptions, you should always use the try/finally mechanism for locking. Here, for example, it's possible that the addition may throw an exception (even though you don't catch it):
public void addAmount(double amount)
this.accountLock.writeLock().lock();
try
this.balance.add(new BigDecimal(amount));
finally
this.accountLock.writeLock().unlock();
In case you think that's extreme, well, the amount could be NaN
or infinity
, and that would throw a NumberFormatException
, etc. Even if it were impossible for the logic to throw an error, you should still use the try/finally mechanism because it makes the logic obvious.
The balance method has the most to gain:
public double getBalance()
this.accountLock.readLock().lock();
try
return this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
finally
this.accountLock.readLock().unlock();
You are also leaking the lock through the public method to get it. You really should not allow other people to manipulate the lock strategy you have in your class. It is intended to be internal for a reason.
Speaking of that lock, you should also make it final...
private final ReadWriteLock accountLock;
Bugs
The most glaring issue is not with your locking, but with the balance management itself. BigDecimals are immutable. They cannot be changed. This does nothing: this.balance.add(new BigDecimal(amount));
.... that should be this.balance = this.balance.add(new BigDecimal(amount));
.
The accountNumber
and accountHolder
should be final as well.
Finally, the getBalance method will not always return a 2-decimal double value. Not all values in binary floating-point are representable in decimal.
$endgroup$
In terms of the basic thread locking, it looks like it is doing the right thing, but there are a number of issues in how you are calculating the account balance, and also some escaped locking as well.
Note, your post is titled "Synchronized implementation", but it is not, it is a locked implementation. Synchronization is different, and, in this case, it may be a simpler mechanism.
Locking
Even if you don't catch exceptions, you should always use the try/finally mechanism for locking. Here, for example, it's possible that the addition may throw an exception (even though you don't catch it):
public void addAmount(double amount)
this.accountLock.writeLock().lock();
try
this.balance.add(new BigDecimal(amount));
finally
this.accountLock.writeLock().unlock();
In case you think that's extreme, well, the amount could be NaN
or infinity
, and that would throw a NumberFormatException
, etc. Even if it were impossible for the logic to throw an error, you should still use the try/finally mechanism because it makes the logic obvious.
The balance method has the most to gain:
public double getBalance()
this.accountLock.readLock().lock();
try
return this.balance.setScale(2, RoundingMode.HALF_DOWN).doubleValue();
finally
this.accountLock.readLock().unlock();
You are also leaking the lock through the public method to get it. You really should not allow other people to manipulate the lock strategy you have in your class. It is intended to be internal for a reason.
Speaking of that lock, you should also make it final...
private final ReadWriteLock accountLock;
Bugs
The most glaring issue is not with your locking, but with the balance management itself. BigDecimals are immutable. They cannot be changed. This does nothing: this.balance.add(new BigDecimal(amount));
.... that should be this.balance = this.balance.add(new BigDecimal(amount));
.
The accountNumber
and accountHolder
should be final as well.
Finally, the getBalance method will not always return a 2-decimal double value. Not all values in binary floating-point are representable in decimal.
edited 11 hours ago
answered 11 hours ago
rolfl♦rolfl
91.1k13192395
91.1k13192395
add a comment |
add a comment |
$begingroup$
double
is not a good choice to use for currency in Java. The better option is BigDecimal
(which you are using for the internal balance, but not for the parameters passed to the addAmount
and withdrawAmount
methods). A better approach would be to make those methods take a BigDecimal
parameter instead (and to use BigDecimal
everywhere in your code that deals with currency amounts).
If for some reason those methods need to take a double
parameter then you should not convert it to a BigDecimal
with the new BigDecimal(double)
constructor - this will give an inaccurate conversion and an unexpected (and incorrect) value for the balance after the add/withdraw operation. For example, the following test fails:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(new BigDecimal(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
with the error
java.lang.AssertionError:
Expected: is <0.1>
but: was <0.1000000000000000055511151231257827021181583404541015625>
The correct way to convert from a double
to a BigDecimal
is to use BigDecimal.valueOf(double)
. For example, changing the middle line in the above test will make it pass:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(BigDecimal.valueOf(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
New contributor
$endgroup$
add a comment |
$begingroup$
double
is not a good choice to use for currency in Java. The better option is BigDecimal
(which you are using for the internal balance, but not for the parameters passed to the addAmount
and withdrawAmount
methods). A better approach would be to make those methods take a BigDecimal
parameter instead (and to use BigDecimal
everywhere in your code that deals with currency amounts).
If for some reason those methods need to take a double
parameter then you should not convert it to a BigDecimal
with the new BigDecimal(double)
constructor - this will give an inaccurate conversion and an unexpected (and incorrect) value for the balance after the add/withdraw operation. For example, the following test fails:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(new BigDecimal(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
with the error
java.lang.AssertionError:
Expected: is <0.1>
but: was <0.1000000000000000055511151231257827021181583404541015625>
The correct way to convert from a double
to a BigDecimal
is to use BigDecimal.valueOf(double)
. For example, changing the middle line in the above test will make it pass:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(BigDecimal.valueOf(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
New contributor
$endgroup$
add a comment |
$begingroup$
double
is not a good choice to use for currency in Java. The better option is BigDecimal
(which you are using for the internal balance, but not for the parameters passed to the addAmount
and withdrawAmount
methods). A better approach would be to make those methods take a BigDecimal
parameter instead (and to use BigDecimal
everywhere in your code that deals with currency amounts).
If for some reason those methods need to take a double
parameter then you should not convert it to a BigDecimal
with the new BigDecimal(double)
constructor - this will give an inaccurate conversion and an unexpected (and incorrect) value for the balance after the add/withdraw operation. For example, the following test fails:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(new BigDecimal(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
with the error
java.lang.AssertionError:
Expected: is <0.1>
but: was <0.1000000000000000055511151231257827021181583404541015625>
The correct way to convert from a double
to a BigDecimal
is to use BigDecimal.valueOf(double)
. For example, changing the middle line in the above test will make it pass:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(BigDecimal.valueOf(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
New contributor
$endgroup$
double
is not a good choice to use for currency in Java. The better option is BigDecimal
(which you are using for the internal balance, but not for the parameters passed to the addAmount
and withdrawAmount
methods). A better approach would be to make those methods take a BigDecimal
parameter instead (and to use BigDecimal
everywhere in your code that deals with currency amounts).
If for some reason those methods need to take a double
parameter then you should not convert it to a BigDecimal
with the new BigDecimal(double)
constructor - this will give an inaccurate conversion and an unexpected (and incorrect) value for the balance after the add/withdraw operation. For example, the following test fails:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(new BigDecimal(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
with the error
java.lang.AssertionError:
Expected: is <0.1>
but: was <0.1000000000000000055511151231257827021181583404541015625>
The correct way to convert from a double
to a BigDecimal
is to use BigDecimal.valueOf(double)
. For example, changing the middle line in the above test will make it pass:
@Test
public void demonstrateBigDecimalRoundingErrorsFromDouble()
BigDecimal balance = BigDecimal.ZERO;
balance = balance.add(BigDecimal.valueOf(0.1));
assertThat(balance, is(new BigDecimal("0.1")));
New contributor
New contributor
answered 2 hours ago
Player OnePlayer One
1011
1011
New contributor
New contributor
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f215616%2fsynchronized-implementation-of-a-bank-account-in-java%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown