Why do I not see the transactions when opening a customer

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

Why do I not see the transactions when opening a customer

Roelof Wobben
Hello,

I try now to make this challege work on Pharo which is orginal a c# problem to practice OOP.

The challenge is this :

Consider the following situation:
You have a bank account. Bank account has a password which is hidden and you can access data within bank 
account only by providing the right credentials.
Bank acount has a whole bunch of transaction history. 
Transaction is basically a record with name, description, amount (either positive or negative) 
and specific account linked to it (your bank accounts can have multiple accounts for money). 

You need to: ask each user for credentials and output how much money do they have in each bank account.

Classes: Customer, Transaction, BankAccount.

So far I have this : https://github.com/rwobben/bankaccount

but when I do this on playground:

customer := Customer new. 
bankAccount := Bankaccounts new.
bankAccount password: 'secret'.
customer AddBankAccountToCustomer: bankAccount.

transaction = Transactions new. 
bankAccount addToBankAccount:  transaction.

customer. 


and I inspect the customer I do not see the transaction addded to the bankaccounts collection. 

What do I do wrong ? 

Roelof
 


Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Ben Coman
and I inspect the customer I do not see the transaction added to the bankaccounts collection. 
 Rather than focus on what you don't see.  What do you see?  And does anything look odd about it?

cheers -ben
Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Op 16-8-2019 om 15:32 schreef Ben Coman:
and I inspect the customer I do not see the transaction added to the bankaccounts collection. 
 Rather than focus on what you don't see.  What do you see?  And does anything look odd about it?

cheers -ben

I see that customer has a BankAccounts list which has 10 possible entries

and that does not look odd to me.

Roelof

Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Richard O'Keefe
In reply to this post by Roelof Wobben
What did you expect
    transaction = Transactions new.
to do, besides computing the answer 'false' and throwing it away?


On Fri, 16 Aug 2019 at 22:38, Roelof Wobben <[hidden email]> wrote:
Hello,

I try now to make this challege work on Pharo which is orginal a c# problem to practice OOP.

The challenge is this :

Consider the following situation:
You have a bank account. Bank account has a password which is hidden and you can access data within bank 
account only by providing the right credentials.
Bank acount has a whole bunch of transaction history. 
Transaction is basically a record with name, description, amount (either positive or negative) 
and specific account linked to it (your bank accounts can have multiple accounts for money). 

You need to: ask each user for credentials and output how much money do they have in each bank account.

Classes: Customer, Transaction, BankAccount.

So far I have this : https://github.com/rwobben/bankaccount

but when I do this on playground:

customer := Customer new. 
bankAccount := Bankaccounts new.
bankAccount password: 'secret'.
customer AddBankAccountToCustomer: bankAccount.

transaction = Transactions new. 
bankAccount addToBankAccount:  transaction.

customer. 


and I inspect the customer I do not see the transaction addded to the bankaccounts collection. 

What do I do wrong ? 

Roelof
 


Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Richard O'Keefe
In reply to this post by Roelof Wobben
What is the source of the challenge?
Is there a version in English?
I am having very serious trouble trying to make sense of
"your bank accounts can have multiple accounts for money".

I note that in your C# code a customer has an ID, a first name,
and a last name.  There is nothing about this in the challenge
text, and since some people have one-part names, I have a three-
part name, and my father had and my grand-daughter has four-part
names, I should think that simply representing a name as a string
without caring about subdivisions would be better.  What I'm
getting at is this: with a specification like that, how will you
know when your code is correct?
Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Op 17-8-2019 om 07:16 schreef Richard O'Keefe:
What is the source of the challenge?
Is there a version in English?
I am having very serious trouble trying to make sense of
"your bank accounts can have multiple accounts for money".

I note that in your C# code a customer has an ID, a first name,
and a last name.  There is nothing about this in the challenge
text, and since some people have one-part names, I have a three-
part name, and my father had and my grand-daughter has four-part
names, I should think that simply representing a name as a string
without caring about subdivisions would be better.  What I'm
getting at is this: with a specification like that, how will you
know when your code is correct?

The source is one of the mentors of a c# learning channel made it up to practice OOP.

I think you never known.

Roelof

Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Richard O'Keefe
points to a repository with C# code, so I cannot comment on the Smalltalk.
There is, however, a striking thing about the C# code, qua C#, that is
worth mentioning.

It is not encapsulated.

Let's start with the most obvious one.
A bank account has a password, and you are
supposed to provide the right password in order
to get information from it.

Your interface *reveals* the password, so
malicious code can do
   var pass = theBankAccount.password;
and then provide the password back.  What's
needed is something like

  var balance = theBankAccount.queryBalance(pass);

where passwords go *in* to theBankAccount but never ever
ever come *out*.

Another thing is that a BankAccount belongs to a specific
Customer.  There's a good way and a bad way to manage this.
The bad way is for the caller to create a new account that
belongs to nobody, and then *tell* the Customer "here is a
new account".  With this interface, you could tell any
number of Customers that they own the account.  That's
possible in the real world, but in the challenge, it isn't.
The good way is to *ask* the Customer "please create a new
account and tell me what it is", and have no way to force
a Customer to accept an account willy nilly.  That way a
Customer can be certain that the accounts it is holding
belong to it and it alone.

Or it could be were it not for a major encapsulation issue.
The idea of encapsulation is that
  EVERY CHANGE TO AN OBJECT'S STATE MUST BE MADE BY THAT OBJECT
including the start of owned containers.

In Java, for example, you might have
   public class Customer {
      private final ArrayList<Account> accounts = new ArrayList<Account>();
      ...
      public List<Account> getAccounts() {
         return Collections.unmodifiableList(accounts);
      }
      ...
      public Account newAccount(...details...) {
         final Account a = new Account(...details...');
         accounts.add(a);
         return a;
      }
      ...
   }
If you are going to provide access to a collection,
(1) make the collection immutable in the first place, or
(2) return an immutable view of it, or
(3) best of all, DON'T provide access to the collection
but provide queries that extract just the information you
want to make available.

This has nothing to do with Java or Smalltalk or C# as such;
encapsulation is object-oriented-programming 102.

For what it's worth, I used to be a University lecturer, and
it was MUCH harder for students to grasp encapsulation than
to grasp recursion.  I have no idea why.  Just pretend that you
are a nice little object surrounded by malevolent pranksters
who can force you to do anything in your interface, and make
sure they cannot harm you.

Oh, when I asked about the source of the challenge, what I was
getting at was "is this your paraphrase of the challenge, in
which case please tell us to find the original, or is this
the actual unmodified original text, in which case --ing h---,
you have my profound sympathy."

This looks like a garbled version of an exercise that I did
a little over 10 years ago, which I found in a Java-based book
whose details I unfortunately forgot to record.  Right down to
the names of the classes!  That is why I am pretty sure that
there is a description of the problem somewhere that makes sense.

Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Sorry, then a pointed to a wrong repo
this is the  repo with smalltalk code :

https://github.com/rwobben/banking

Roelof



Op 17-8-2019 om 11:25 schreef Richard O'Keefe:
points to a repository with C# code, so I cannot comment on the Smalltalk.
There is, however, a striking thing about the C# code, qua C#, that is
worth mentioning.

It is not encapsulated.

Let's start with the most obvious one.
A bank account has a password, and you are
supposed to provide the right password in order
to get information from it.

Your interface *reveals* the password, so
malicious code can do
   var pass = theBankAccount.password;
and then provide the password back.  What's
needed is something like

  var balance = theBankAccount.queryBalance(pass);

where passwords go *in* to theBankAccount but never ever
ever come *out*.

Another thing is that a BankAccount belongs to a specific
Customer.  There's a good way and a bad way to manage this.
The bad way is for the caller to create a new account that
belongs to nobody, and then *tell* the Customer "here is a
new account".  With this interface, you could tell any
number of Customers that they own the account.  That's
possible in the real world, but in the challenge, it isn't.
The good way is to *ask* the Customer "please create a new
account and tell me what it is", and have no way to force
a Customer to accept an account willy nilly.  That way a
Customer can be certain that the accounts it is holding
belong to it and it alone.

Or it could be were it not for a major encapsulation issue.
The idea of encapsulation is that
  EVERY CHANGE TO AN OBJECT'S STATE MUST BE MADE BY THAT OBJECT
including the start of owned containers.

In Java, for example, you might have
   public class Customer {
      private final ArrayList<Account> accounts = new ArrayList<Account>();
      ...
      public List<Account> getAccounts() {
         return Collections.unmodifiableList(accounts);
      }
      ...
      public Account newAccount(...details...) {
         final Account a = new Account(...details...');
         accounts.add(a);
         return a;
      }
      ...
   }
If you are going to provide access to a collection,
(1) make the collection immutable in the first place, or
(2) return an immutable view of it, or
(3) best of all, DON'T provide access to the collection
but provide queries that extract just the information you
want to make available.

This has nothing to do with Java or Smalltalk or C# as such;
encapsulation is object-oriented-programming 102.

For what it's worth, I used to be a University lecturer, and
it was MUCH harder for students to grasp encapsulation than
to grasp recursion.  I have no idea why.  Just pretend that you
are a nice little object surrounded by malevolent pranksters
who can force you to do anything in your interface, and make
sure they cannot harm you.

Oh, when I asked about the source of the challenge, what I was
getting at was "is this your paraphrase of the challenge, in
which case please tell us to find the original, or is this
the actual unmodified original text, in which case --ing h---,
you have my profound sympathy."

This looks like a garbled version of an exercise that I did
a little over 10 years ago, which I found in a Java-based book
whose details I unfortunately forgot to record.  Right down to
the names of the classes!  That is why I am pretty sure that
there is a description of the problem somewhere that makes sense.


Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Richard O'Keefe
Good-oh.
Looking at Bankaccounts
 - the name should be singular, not plural
 - Smalltalk uses baStudlyCaps
 > Call it BankAccount.

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).

 - #addToBankAccount: is an unhelpful name; we
   know it's adding to a BankAccount because that
   is where it is.  But what is it adding?
   (Hint: an account might have more than one
   customer in the real world, so we might need
   to distinguish between #addCustomer: and)
 > #addTransaction: , which would be a better name.

 - There is a missing method.  When you ask a bank
   account to report a secret, *it* should check
   the password.
 > add
    queryBalance: pwd
      ^password = pwd
         ifTrue:  [transactions detectSum: [:each | each amount]]
         ifFalse: [self error: 'wrong password']


Transactions
 - This too should be singular, not plural.
 > Rename it to Transaction.

 - You know it is unfinished.

Customer
 - You know it is unfinished.

 - Smalltalk uses baStudlyCaps style.  The instance
   variable 'bankaccounts' is two words run together.
 > Change it to 'bankAccounts'.

 - The very first method has two style issues and a major flaw.
   AddBankAccountToCustomer: bankaccount [
     bankaccounts add: bankaccounts
   ]
 - Style flaw one: method names are expected to begin
   with a lower case letter, not a capital.  (That's C#.)
 - Style flaw two; we *knew* it's adding to a customer,
   because that's where it is.
 > Rename to #addBankAccount:

 - The major flaw is that the argument 'bankaccount' is
   ignored, while the 'bankaccounts' collection is added
   TO ITSELF.  This is one of the rare occasions when a
   type checker would have caught a mistake.
 > Revise to
   addBankAccount:aBankAccount [
     bankAccounts addLast: aBankAccount.
   ]

 - You have a setter, #bankaccounts:.  WHY?  This should be
   a completely private variable, initialised when the
   Customer object is created, and never reassigned thereafter.
 > Delete that setter.


Customer >> AddBankAccountToCustomer: bankaccount [ 
   bankaccounts  add:  bankaccounts.

]

On Sat, 17 Aug 2019 at 21:59, Roelof Wobben <[hidden email]> wrote:
Sorry, then a pointed to a wrong repo
this is the  repo with smalltalk code :

https://github.com/rwobben/banking

Roelof



Op 17-8-2019 om 11:25 schreef Richard O'Keefe:
points to a repository with C# code, so I cannot comment on the Smalltalk.
There is, however, a striking thing about the C# code, qua C#, that is
worth mentioning.

It is not encapsulated.

Let's start with the most obvious one.
A bank account has a password, and you are
supposed to provide the right password in order
to get information from it.

Your interface *reveals* the password, so
malicious code can do
   var pass = theBankAccount.password;
and then provide the password back.  What's
needed is something like

  var balance = theBankAccount.queryBalance(pass);

where passwords go *in* to theBankAccount but never ever
ever come *out*.

Another thing is that a BankAccount belongs to a specific
Customer.  There's a good way and a bad way to manage this.
The bad way is for the caller to create a new account that
belongs to nobody, and then *tell* the Customer "here is a
new account".  With this interface, you could tell any
number of Customers that they own the account.  That's
possible in the real world, but in the challenge, it isn't.
The good way is to *ask* the Customer "please create a new
account and tell me what it is", and have no way to force
a Customer to accept an account willy nilly.  That way a
Customer can be certain that the accounts it is holding
belong to it and it alone.

Or it could be were it not for a major encapsulation issue.
The idea of encapsulation is that
  EVERY CHANGE TO AN OBJECT'S STATE MUST BE MADE BY THAT OBJECT
including the start of owned containers.

In Java, for example, you might have
   public class Customer {
      private final ArrayList<Account> accounts = new ArrayList<Account>();
      ...
      public List<Account> getAccounts() {
         return Collections.unmodifiableList(accounts);
      }
      ...
      public Account newAccount(...details...) {
         final Account a = new Account(...details...');
         accounts.add(a);
         return a;
      }
      ...
   }
If you are going to provide access to a collection,
(1) make the collection immutable in the first place, or
(2) return an immutable view of it, or
(3) best of all, DON'T provide access to the collection
but provide queries that extract just the information you
want to make available.

This has nothing to do with Java or Smalltalk or C# as such;
encapsulation is object-oriented-programming 102.

For what it's worth, I used to be a University lecturer, and
it was MUCH harder for students to grasp encapsulation than
to grasp recursion.  I have no idea why.  Just pretend that you
are a nice little object surrounded by malevolent pranksters
who can force you to do anything in your interface, and make
sure they cannot harm you.

Oh, when I asked about the source of the challenge, what I was
getting at was "is this your paraphrase of the challenge, in
which case please tell us to find the original, or is this
the actual unmodified original text, in which case --ing h---,
you have my profound sympathy."

This looks like a garbled version of an exercise that I did
a little over 10 years ago, which I found in a Java-based book
whose details I unfortunately forgot to record.  Right down to
the names of the classes!  That is why I am pretty sure that
there is a description of the problem somewhere that makes sense.


Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Op 17-8-2019 om 12:33 schreef Richard O'Keefe:
Good-oh.
Looking at Bankaccounts
 - the name should be singular, not plural
 - Smalltalk uses baStudlyCaps
 > Call it BankAccount.

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).


Thanks all changed.

 - #addToBankAccount: is an unhelpful name; we
   know it's adding to a BankAccount because that
   is where it is.  But what is it adding?
   (Hint: an account might have more than one
   customer in the real world, so we might need
   to distinguish between #addCustomer: and)
 > #addTransaction: , which would be a better name.


the method adds a bankaccount to the bankAccounts collection  so maybe AddBankAccountToBankAccountsOfCustomer ?

 - There is a missing method.  When you ask a bank
   account to report a secret, *it* should check
   the password.
 > add
    queryBalance: pwd
      ^password = pwd
         ifTrue:  [transactions detectSum: [:each | each amount]]
         ifFalse: [self error: 'wrong password']


I know its missing. That is the part I still have to figure out.How can I make a collection of all transactions split by account and then the total of it.


Transactions
 - This too should be singular, not plural.
 > Rename it to Transaction.

 - You know it is unfinished.



Changed the name and what is missing here then.


Customer
 - You know it is unfinished.


What is then missing here.

 - Smalltalk uses baStudlyCaps style.  The instance
   variable 'bankaccounts' is two words run together.
 > Change it to 'bankAccounts'.

 - The very first method has two style issues and a major flaw.
   AddBankAccountToCustomer: bankaccount [
     bankaccounts add: bankaccounts
   ]
 - Style flaw one: method names are expected to begin
   with a lower case letter, not a capital.  (That's C#.)
 - Style flaw two; we *knew* it's adding to a customer,
   because that's where it is.
 > Rename to #addBankAccount:

 - The major flaw is that the argument 'bankaccount' is
   ignored, while the 'bankaccounts' collection is added
   TO ITSELF.  This is one of the rare occasions when a
   type checker would have caught a mistake.
 > Revise to
   addBankAccount:aBankAccount [
     bankAccounts addLast: aBankAccount.
   ]

 - You have a setter, #bankaccounts:.  WHY?  This should be
   a completely private variable, initialised when the
   Customer object is created, and never reassigned thereafter.
 > Delete that setter.


 Done and Done.


Customer >> AddBankAccountToCustomer: bankaccount [ 
   bankaccounts  add:  bankaccounts.

]

On Sat, 17 Aug 2019 at 21:59, Roelof Wobben <[hidden email]> wrote:
Sorry, then a pointed to a wrong repo
this is the  repo with smalltalk code :

https://github.com/rwobben/banking

Roelof



Op 17-8-2019 om 11:25 schreef Richard O'Keefe:
points to a repository with C# code, so I cannot comment on the Smalltalk.
There is, however, a striking thing about the C# code, qua C#, that is
worth mentioning.

It is not encapsulated.

Let's start with the most obvious one.
A bank account has a password, and you are
supposed to provide the right password in order
to get information from it.

Your interface *reveals* the password, so
malicious code can do
   var pass = theBankAccount.password;
and then provide the password back.  What's
needed is something like

  var balance = theBankAccount.queryBalance(pass);

where passwords go *in* to theBankAccount but never ever
ever come *out*.

Another thing is that a BankAccount belongs to a specific
Customer.  There's a good way and a bad way to manage this.
The bad way is for the caller to create a new account that
belongs to nobody, and then *tell* the Customer "here is a
new account".  With this interface, you could tell any
number of Customers that they own the account.  That's
possible in the real world, but in the challenge, it isn't.
The good way is to *ask* the Customer "please create a new
account and tell me what it is", and have no way to force
a Customer to accept an account willy nilly.  That way a
Customer can be certain that the accounts it is holding
belong to it and it alone.

Or it could be were it not for a major encapsulation issue.
The idea of encapsulation is that
  EVERY CHANGE TO AN OBJECT'S STATE MUST BE MADE BY THAT OBJECT
including the start of owned containers.

In Java, for example, you might have
   public class Customer {
      private final ArrayList<Account> accounts = new ArrayList<Account>();
      ...
      public List<Account> getAccounts() {
         return Collections.unmodifiableList(accounts);
      }
      ...
      public Account newAccount(...details...) {
         final Account a = new Account(...details...');
         accounts.add(a);
         return a;
      }
      ...
   }
If you are going to provide access to a collection,
(1) make the collection immutable in the first place, or
(2) return an immutable view of it, or
(3) best of all, DON'T provide access to the collection
but provide queries that extract just the information you
want to make available.

This has nothing to do with Java or Smalltalk or C# as such;
encapsulation is object-oriented-programming 102.

For what it's worth, I used to be a University lecturer, and
it was MUCH harder for students to grasp encapsulation than
to grasp recursion.  I have no idea why.  Just pretend that you
are a nice little object surrounded by malevolent pranksters
who can force you to do anything in your interface, and make
sure they cannot harm you.

Oh, when I asked about the source of the challenge, what I was
getting at was "is this your paraphrase of the challenge, in
which case please tell us to find the original, or is this
the actual unmodified original text, in which case --ing h---,
you have my profound sympathy."

This looks like a garbled version of an exercise that I did
a little over 10 years ago, which I found in a Java-based book
whose details I unfortunately forgot to record.  Right down to
the names of the classes!  That is why I am pretty sure that
there is a description of the problem somewhere that makes sense.



Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Thanks,

I implement almost everything now.

But I fail to see how to make for password a private variable without getters and setters
and still  have different password for every bankaccount and can test against it.

So please help.

Roelof



Op 17-8-2019 om 12:48 schreef Roelof Wobben:
Op 17-8-2019 om 12:33 schreef Richard O'Keefe:
Good-oh.
Looking at Bankaccounts
 - the name should be singular, not plural
 - Smalltalk uses baStudlyCaps
 > Call it BankAccount.

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).


Thanks all changed.

 - #addToBankAccount: is an unhelpful name; we
   know it's adding to a BankAccount because that
   is where it is.  But what is it adding?
   (Hint: an account might have more than one
   customer in the real world, so we might need
   to distinguish between #addCustomer: and)
 > #addTransaction: , which would be a better name.


the method adds a bankaccount to the bankAccounts collection  so maybe AddBankAccountToBankAccountsOfCustomer ?

 - There is a missing method.  When you ask a bank
   account to report a secret, *it* should check
   the password.
 > add
    queryBalance: pwd
      ^password = pwd
         ifTrue:  [transactions detectSum: [:each | each amount]]
         ifFalse: [self error: 'wrong password']


I know its missing. That is the part I still have to figure out.How can I make a collection of all transactions split by account and then the total of it.


Transactions
 - This too should be singular, not plural.
 > Rename it to Transaction.

 - You know it is unfinished.



Changed the name and what is missing here then.


Customer
 - You know it is unfinished.


What is then missing here.

 - Smalltalk uses baStudlyCaps style.  The instance
   variable 'bankaccounts' is two words run together.
 > Change it to 'bankAccounts'.

 - The very first method has two style issues and a major flaw.
   AddBankAccountToCustomer: bankaccount [
     bankaccounts add: bankaccounts
   ]
 - Style flaw one: method names are expected to begin
   with a lower case letter, not a capital.  (That's C#.)
 - Style flaw two; we *knew* it's adding to a customer,
   because that's where it is.
 > Rename to #addBankAccount:

 - The major flaw is that the argument 'bankaccount' is
   ignored, while the 'bankaccounts' collection is added
   TO ITSELF.  This is one of the rare occasions when a
   type checker would have caught a mistake.
 > Revise to
   addBankAccount:aBankAccount [
     bankAccounts addLast: aBankAccount.
   ]

 - You have a setter, #bankaccounts:.  WHY?  This should be
   a completely private variable, initialised when the
   Customer object is created, and never reassigned thereafter.
 > Delete that setter.


 Done and Done.


Customer >> AddBankAccountToCustomer: bankaccount [ 
   bankaccounts  add:  bankaccounts.

]

On Sat, 17 Aug 2019 at 21:59, Roelof Wobben <[hidden email]> wrote:
Sorry, then a pointed to a wrong repo
this is the  repo with smalltalk code :

https://github.com/rwobben/banking

Roelof



Op 17-8-2019 om 11:25 schreef Richard O'Keefe:
points to a repository with C# code, so I cannot comment on the Smalltalk.
There is, however, a striking thing about the C# code, qua C#, that is
worth mentioning.

It is not encapsulated.

Let's start with the most obvious one.
A bank account has a password, and you are
supposed to provide the right password in order
to get information from it.

Your interface *reveals* the password, so
malicious code can do
   var pass = theBankAccount.password;
and then provide the password back.  What's
needed is something like

  var balance = theBankAccount.queryBalance(pass);

where passwords go *in* to theBankAccount but never ever
ever come *out*.

Another thing is that a BankAccount belongs to a specific
Customer.  There's a good way and a bad way to manage this.
The bad way is for the caller to create a new account that
belongs to nobody, and then *tell* the Customer "here is a
new account".  With this interface, you could tell any
number of Customers that they own the account.  That's
possible in the real world, but in the challenge, it isn't.
The good way is to *ask* the Customer "please create a new
account and tell me what it is", and have no way to force
a Customer to accept an account willy nilly.  That way a
Customer can be certain that the accounts it is holding
belong to it and it alone.

Or it could be were it not for a major encapsulation issue.
The idea of encapsulation is that
  EVERY CHANGE TO AN OBJECT'S STATE MUST BE MADE BY THAT OBJECT
including the start of owned containers.

In Java, for example, you might have
   public class Customer {
      private final ArrayList<Account> accounts = new ArrayList<Account>();
      ...
      public List<Account> getAccounts() {
         return Collections.unmodifiableList(accounts);
      }
      ...
      public Account newAccount(...details...) {
         final Account a = new Account(...details...');
         accounts.add(a);
         return a;
      }
      ...
   }
If you are going to provide access to a collection,
(1) make the collection immutable in the first place, or
(2) return an immutable view of it, or
(3) best of all, DON'T provide access to the collection
but provide queries that extract just the information you
want to make available.

This has nothing to do with Java or Smalltalk or C# as such;
encapsulation is object-oriented-programming 102.

For what it's worth, I used to be a University lecturer, and
it was MUCH harder for students to grasp encapsulation than
to grasp recursion.  I have no idea why.  Just pretend that you
are a nice little object surrounded by malevolent pranksters
who can force you to do anything in your interface, and make
sure they cannot harm you.

Oh, when I asked about the source of the challenge, what I was
getting at was "is this your paraphrase of the challenge, in
which case please tell us to find the original, or is this
the actual unmodified original text, in which case --ing h---,
you have my profound sympathy."

This looks like a garbled version of an exercise that I did
a little over 10 years ago, which I found in a Java-based book
whose details I unfortunately forgot to record.  Right down to
the names of the classes!  That is why I am pretty sure that
there is a description of the problem somewhere that makes sense.




Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

DiegoLont
The best way to implement a password security is to never store the password, but only a hashed password. This way, you never can have a security leak for your passwords: because you don’t have them. And for hashing you use a standard modern hashing algorithm, so it cannot be easily rolled back. What I often use is that I make the username readonly, and make the hash depend on the username (i.e. use the username as seed)

So you implement
password: aString
self hashedPassword: (self hash: aString)

verifyPassword: aString
^(self hash: aString) = self hashedPassword

Regards,
Diego

On 18 Aug 2019, at 18:23, Roelof Wobben <[hidden email]> wrote:

Thanks,

I implement almost everything now.

But I fail to see how to make for password a private variable without getters and setters
and still  have different password for every bankaccount and can test against it.

So please help.

Roelof



Op 17-8-2019 om 12:48 schreef Roelof Wobben:
Op 17-8-2019 om 12:33 schreef Richard O'Keefe:
Good-oh.
Looking at Bankaccounts
 - the name should be singular, not plural
 - Smalltalk uses baStudlyCaps
 > Call it BankAccount.

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).


Thanks all changed.

 - #addToBankAccount: is an unhelpful name; we
   know it's adding to a BankAccount because that
   is where it is.  But what is it adding?
   (Hint: an account might have more than one
   customer in the real world, so we might need
   to distinguish between #addCustomer: and)
 > #addTransaction: , which would be a better name.


the method adds a bankaccount to the bankAccounts collection  so maybe AddBankAccountToBankAccountsOfCustomer ?

 - There is a missing method.  When you ask a bank
   account to report a secret, *it* should check
   the password.
 > add
    queryBalance: pwd
      ^password = pwd
         ifTrue:  [transactions detectSum: [:each | each amount]]
         ifFalse: [self error: 'wrong password']


I know its missing. That is the part I still have to figure out.How can I make a collection of all transactions split by account and then the total of it.


Transactions
 - This too should be singular, not plural.
 > Rename it to Transaction.

 - You know it is unfinished.



Changed the name and what is missing here then.


Customer
 - You know it is unfinished.


What is then missing here.

 - Smalltalk uses baStudlyCaps style.  The instance
   variable 'bankaccounts' is two words run together.
 > Change it to 'bankAccounts'.

 - The very first method has two style issues and a major flaw.
   AddBankAccountToCustomer: bankaccount [
     bankaccounts add: bankaccounts
   ]
 - Style flaw one: method names are expected to begin
   with a lower case letter, not a capital.  (That's C#.)
 - Style flaw two; we *knew* it's adding to a customer,
   because that's where it is.
 > Rename to #addBankAccount:

 - The major flaw is that the argument 'bankaccount' is
   ignored, while the 'bankaccounts' collection is added
   TO ITSELF.  This is one of the rare occasions when a
   type checker would have caught a mistake.
 > Revise to
   addBankAccount:aBankAccount [
     bankAccounts addLast: aBankAccount.
   ]

 - You have a setter, #bankaccounts:.  WHY?  This should be
   a completely private variable, initialised when the
   Customer object is created, and never reassigned thereafter.
 > Delete that setter.


 Done and Done.


Customer >> AddBankAccountToCustomer: bankaccount [ 
   bankaccounts  add:  bankaccounts.

]

On Sat, 17 Aug 2019 at 21:59, Roelof Wobben <[hidden email]> wrote:
Sorry, then a pointed to a wrong repo
this is the  repo with smalltalk code :

https://github.com/rwobben/banking

Roelof



Op 17-8-2019 om 11:25 schreef Richard O'Keefe:
points to a repository with C# code, so I cannot comment on the Smalltalk.
There is, however, a striking thing about the C# code, qua C#, that is
worth mentioning.

It is not encapsulated.

Let's start with the most obvious one.
A bank account has a password, and you are
supposed to provide the right password in order
to get information from it.

Your interface *reveals* the password, so
malicious code can do
   var pass = theBankAccount.password;
and then provide the password back.  What's
needed is something like

  var balance = theBankAccount.queryBalance(pass);

where passwords go *in* to theBankAccount but never ever
ever come *out*.

Another thing is that a BankAccount belongs to a specific
Customer.  There's a good way and a bad way to manage this.
The bad way is for the caller to create a new account that
belongs to nobody, and then *tell* the Customer "here is a
new account".  With this interface, you could tell any
number of Customers that they own the account.  That's
possible in the real world, but in the challenge, it isn't.
The good way is to *ask* the Customer "please create a new
account and tell me what it is", and have no way to force
a Customer to accept an account willy nilly.  That way a
Customer can be certain that the accounts it is holding
belong to it and it alone.

Or it could be were it not for a major encapsulation issue.
The idea of encapsulation is that
  EVERY CHANGE TO AN OBJECT'S STATE MUST BE MADE BY THAT OBJECT
including the start of owned containers.

In Java, for example, you might have
   public class Customer {
      private final ArrayList<Account> accounts = new ArrayList<Account>();
      ...
      public List<Account> getAccounts() {
         return Collections.unmodifiableList(accounts);
      }
      ...
      public Account newAccount(...details...) {
         final Account a = new Account(...details...');
         accounts.add(a);
         return a;
      }
      ...
   }
If you are going to provide access to a collection,
(1) make the collection immutable in the first place, or
(2) return an immutable view of it, or
(3) best of all, DON'T provide access to the collection
but provide queries that extract just the information you
want to make available.

This has nothing to do with Java or Smalltalk or C# as such;
encapsulation is object-oriented-programming 102.

For what it's worth, I used to be a University lecturer, and
it was MUCH harder for students to grasp encapsulation than
to grasp recursion.  I have no idea why.  Just pretend that you
are a nice little object surrounded by malevolent pranksters
who can force you to do anything in your interface, and make
sure they cannot harm you.

Oh, when I asked about the source of the challenge, what I was
getting at was "is this your paraphrase of the challenge, in
which case please tell us to find the original, or is this
the actual unmodified original text, in which case --ing h---,
you have my profound sympathy."

This looks like a garbled version of an exercise that I did
a little over 10 years ago, which I found in a Java-based book
whose details I unfortunately forgot to record.  Right down to
the names of the classes!  That is why I am pretty sure that
there is a description of the problem somewhere that makes sense.





Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

JupiterJones
Hi Deigo,

Apologies Roelof for jumping in to your thread. :)

This solves the issue of storing passwords, however, Richard made the case for malevolent or broken code…

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).

To scale there is likely the need to allow customers to change their own password without call centre or administrator intervention.

Do you know of any implementations of user/password stores that are resilient to malevolent or broken code?

Cheers,

J

On 19 Aug 2019, at 5:40 pm, Diego Lont <[hidden email]> wrote:

The best way to implement a password security is to never store the password, but only a hashed password. This way, you never can have a security leak for your passwords: because you don’t have them. And for hashing you use a standard modern hashing algorithm, so it cannot be easily rolled back. What I often use is that I make the username readonly, and make the hash depend on the username (i.e. use the username as seed)

So you implement
password: aString
self hashedPassword: (self hash: aString)

verifyPassword: aString
^(self hash: aString) = self hashedPassword

Regards,
Diego

Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

DiegoLont
Hi Jupiter.

This is always a hard problem, as you state: you want a customer to change their own password, and also want a customer to be allowed to set a password that he has lost. You could of course make your server code more resilient, and demand there is security information provided when changing a password.

I.E.
newPassword: aString authentication: anAuthenticationInformation
“ and here you need to verify the authentication information being:
1. user / password of the user that has the account
2. user / password of an administration
3. reset information sent to the user
… details can very, but as said: there will always be a back door.

So if it is really important, one should implement strong authentication. Strong authentication is based on the fact, that a user not only knows something, but also has something. That is why banks always use things such as a “rabo scanner” that you need to insert your bank pass (something you have), type your pin (something you know) and input a picture / code and the result is a new code that you enter.

On 19 Aug 2019, at 10:01, Jupiter Jones <[hidden email]> wrote:

Hi Deigo,

Apologies Roelof for jumping in to your thread. :)

This solves the issue of storing passwords, however, Richard made the case for malevolent or broken code…

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).

To scale there is likely the need to allow customers to change their own password without call centre or administrator intervention.

Do you know of any implementations of user/password stores that are resilient to malevolent or broken code?

Cheers,

J

On 19 Aug 2019, at 5:40 pm, Diego Lont <[hidden email]> wrote:

The best way to implement a password security is to never store the password, but only a hashed password. This way, you never can have a security leak for your passwords: because you don’t have them. And for hashing you use a standard modern hashing algorithm, so it cannot be easily rolled back. What I often use is that I make the username readonly, and make the hash depend on the username (i.e. use the username as seed)

So you implement
password: aString
self hashedPassword: (self hash: aString)

verifyPassword: aString
^(self hash: aString) = self hashedPassword

Regards,
Diego


Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Thanks all for the answers.

As I see it , it impossible to store the password in the bankaccount object and make safe code to use it,
So some one give me a stupid assignment.

Roelof



Op 19-8-2019 om 11:38 schreef Diego Lont:
Hi Jupiter.

This is always a hard problem, as you state: you want a customer to change their own password, and also want a customer to be allowed to set a password that he has lost. You could of course make your server code more resilient, and demand there is security information provided when changing a password.

I.E.
newPassword: aString authentication: anAuthenticationInformation
“ and here you need to verify the authentication information being:
1. user / password of the user that has the account
2. user / password of an administration
3. reset information sent to the user
… details can very, but as said: there will always be a back door.

So if it is really important, one should implement strong authentication. Strong authentication is based on the fact, that a user not only knows something, but also has something. That is why banks always use things such as a “rabo scanner” that you need to insert your bank pass (something you have), type your pin (something you know) and input a picture / code and the result is a new code that you enter.

On 19 Aug 2019, at 10:01, Jupiter Jones <[hidden email]> wrote:

Hi Deigo,

Apologies Roelof for jumping in to your thread. :)

This solves the issue of storing passwords, however, Richard made the case for malevolent or broken code…

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).

To scale there is likely the need to allow customers to change their own password without call centre or administrator intervention.

Do you know of any implementations of user/password stores that are resilient to malevolent or broken code?

Cheers,

J

On 19 Aug 2019, at 5:40 pm, Diego Lont <[hidden email]> wrote:

The best way to implement a password security is to never store the password, but only a hashed password. This way, you never can have a security leak for your passwords: because you don’t have them. And for hashing you use a standard modern hashing algorithm, so it cannot be easily rolled back. What I often use is that I make the username readonly, and make the hash depend on the username (i.e. use the username as seed)

So you implement
password: aString
self hashedPassword: (self hash: aString)

verifyPassword: aString
^(self hash: aString) = self hashedPassword

Regards,
Diego



Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Ben Coman


On Mon, 19 Aug 2019 at 22:19, Roelof Wobben <[hidden email]> wrote:
Thanks all for the answers.

As I see it , it impossible to store the password in the bankaccount object and make safe code to use it,
So some one give me a stupid assignment.

That would seem to depend on the purpose of the assignment.
  a. To produce production ready code that managed someone's real life money,
  b. Provide a vehicle to reason about typical implementation issues, in which case did you learn anything from the exercise?

cheers -ben

Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Hello Ben,

I think the purpose of this exercise was to practice expecially things like encapsulation and thinking in classes.

So I think it's not a big deal to have a setter and a method to check if the passwords are matching.

Roelof



Op 19-8-2019 om 18:35 schreef Ben Coman:


On Mon, 19 Aug 2019 at 22:19, Roelof Wobben <[hidden email]> wrote:
Thanks all for the answers.

As I see it , it impossible to store the password in the bankaccount object and make safe code to use it,
So some one give me a stupid assignment.

That would seem to depend on the purpose of the assignment.
  a. To produce production ready code that managed someone's real life money,
  b. Provide a vehicle to reason about typical implementation issues, in which case did you learn anything from the exercise?

cheers -ben


Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Richard Sargent
Administrator

On Mon, Aug 19, 2019 at 9:40 AM Roelof Wobben <[hidden email]> wrote:
Hello Ben,

I think the purpose of this exercise was to practice expecially things like encapsulation and thinking in classes.

So I think it's not a big deal to have a setter and a method to check if the passwords are matching.

Roelof,

There are still ways to avoid that. For example, consider a method such as Customer>>addAccountWithPassword: and Account>>#changePasswordFrom:to:. No need for setters and getters and no exposure to unauthorized password changes.
[Of course, this doesn't handle the "I've forgotten my password" scenario, but Account>>#changePasswordTo:usingAuthorizationToken: might.]

I think the exercise is more about APIs than it is about every little detail of an exercise reflecting best practices in the corresponding real world scenario. But, understanding that there are those issues is crucial, as we see almost daily with news of security breaches.

I don't think anyone would argue that completing this exercise should suddenly make you qualified to develop a banking application. :-)



Roelof



Op 19-8-2019 om 18:35 schreef Ben Coman:


On Mon, 19 Aug 2019 at 22:19, Roelof Wobben <[hidden email]> wrote:
Thanks all for the answers.

As I see it , it impossible to store the password in the bankaccount object and make safe code to use it,
So some one give me a stupid assignment.

That would seem to depend on the purpose of the assignment.
  a. To produce production ready code that managed someone's real life money,
  b. Provide a vehicle to reason about typical implementation issues, in which case did you learn anything from the exercise?

cheers -ben


Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Roelof Wobben
Hello,

Thanks for these nice words.

i do not think after this one im a qualified developer , I still have to learn a lot.

Roelof





Op 19-8-2019 om 19:54 schreef Richard Sargent:

On Mon, Aug 19, 2019 at 9:40 AM Roelof Wobben <[hidden email]> wrote:
Hello Ben,

I think the purpose of this exercise was to practice expecially things like encapsulation and thinking in classes.

So I think it's not a big deal to have a setter and a method to check if the passwords are matching.

Roelof,

There are still ways to avoid that. For example, consider a method such as Customer>>addAccountWithPassword: and Account>>#changePasswordFrom:to:. No need for setters and getters and no exposure to unauthorized password changes.
[Of course, this doesn't handle the "I've forgotten my password" scenario, but Account>>#changePasswordTo:usingAuthorizationToken: might.]

I think the exercise is more about APIs than it is about every little detail of an exercise reflecting best practices in the corresponding real world scenario. But, understanding that there are those issues is crucial, as we see almost daily with news of security breaches.

I don't think anyone would argue that completing this exercise should suddenly make you qualified to develop a banking application. :-)



Roelof



Op 19-8-2019 om 18:35 schreef Ben Coman:


On Mon, 19 Aug 2019 at 22:19, Roelof Wobben <[hidden email]> wrote:
Thanks all for the answers.

As I see it , it impossible to store the password in the bankaccount object and make safe code to use it,
So some one give me a stupid assignment.

That would seem to depend on the purpose of the assignment.
  a. To produce production ready code that managed someone's real life money,
  b. Provide a vehicle to reason about typical implementation issues, in which case did you learn anything from the exercise?

cheers -ben



Reply | Threaded
Open this post in threaded view
|

Re: Why do I not see the transactions when opening a customer

Richard O'Keefe
In reply to this post by Roelof Wobben
I think some people are taking this a bit too seriously.
This particular example cannot be taken seriously as an
approximation of a real bank, and the "credentials"
need not be anything other than a simple clear-text string.
There is, for example, *NO* interface of any kind (UI or
API) anywhere in the challenge, and *NO* test cases or
examples from which test cases could be derived.  It is
at best an exercise in not mucking up *too* badly.

Given that, it's not *that* hard to keep secrets, or it
wouldn't be if Pharo didn't have #instVarAt: (:-).  So
the model code I've attached isn't in Pharo (:-).



On Tue, 20 Aug 2019 at 02:19, Roelof Wobben <[hidden email]> wrote:
Thanks all for the answers.

As I see it , it impossible to store the password in the bankaccount object and make safe code to use it,
So some one give me a stupid assignment.

Roelof



Op 19-8-2019 om 11:38 schreef Diego Lont:
Hi Jupiter.

This is always a hard problem, as you state: you want a customer to change their own password, and also want a customer to be allowed to set a password that he has lost. You could of course make your server code more resilient, and demand there is security information provided when changing a password.

I.E.
newPassword: aString authentication: anAuthenticationInformation
“ and here you need to verify the authentication information being:
1. user / password of the user that has the account
2. user / password of an administration
3. reset information sent to the user
… details can very, but as said: there will always be a back door.

So if it is really important, one should implement strong authentication. Strong authentication is based on the fact, that a user not only knows something, but also has something. That is why banks always use things such as a “rabo scanner” that you need to insert your bank pass (something you have), type your pin (something you know) and input a picture / code and the result is a new code that you enter.

On 19 Aug 2019, at 10:01, Jupiter Jones <[hidden email]> wrote:

Hi Deigo,

Apologies Roelof for jumping in to your thread. :)

This solves the issue of storing passwords, however, Richard made the case for malevolent or broken code…

 - you not only provide a getter for the
   password, but a setter!  This means that
   malevolent/broken code can do
     aBankAccount password: 'PWNED!'.
   and then supply the new password 'PWNED!'
   whenever a password is needed.
 > The password must be supplied when a
   BankAccount is changed and it should not
   be settable afterwards (except perhaps with
   a master key).

To scale there is likely the need to allow customers to change their own password without call centre or administrator intervention.

Do you know of any implementations of user/password stores that are resilient to malevolent or broken code?

Cheers,

J

On 19 Aug 2019, at 5:40 pm, Diego Lont <[hidden email]> wrote:

The best way to implement a password security is to never store the password, but only a hashed password. This way, you never can have a security leak for your passwords: because you don’t have them. And for hashing you use a standard modern hashing algorithm, so it cannot be easily rolled back. What I often use is that I make the username readonly, and make the hash depend on the username (i.e. use the username as seed)

So you implement
password: aString
self hashedPassword: (self hash: aString)

verifyPassword: aString
^(self hash: aString) = self hashedPassword

Regards,
Diego




challenge.st (9K) Download Attachment