Skip to content
This repository was archived by the owner on May 18, 2022. It is now read-only.

My design#2

Open
rkyadaviitkgp wants to merge 1 commit intoworkattech:masterfrom
rkyadaviitkgp:master
Open

My design#2
rkyadaviitkgp wants to merge 1 commit intoworkattech:masterfrom
rkyadaviitkgp:master

Conversation

@rkyadaviitkgp
Copy link
Copy Markdown

No description provided.


// List: Each list should have a id, name and cards
private List<Card> cards;
private Board board;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why does ListItem have access to the Board?

}

//We should also be able to move cards across lists in the same board
public void AddCardToList(Card c){
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

camelCase?

this.board = board;
}

public String getPrintableString(){
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can be done by overriding toString

Map<String, ListItem> lists;
Map<String, Card> cards;

public MockMachineCoding3() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

better name?

*/
public enum Privacy {

publicType, privateType;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

enums are generally written in UPPERCASE

@gcnit
Copy link
Copy Markdown
Collaborator

gcnit commented Oct 23, 2019

Hi @rkyadaviitkgp sorry for the delay in code review.

The code looks good. Gave a few comments.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants