Skip to content
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

Loan add backend #345

Merged
merged 34 commits into from
Nov 5, 2019
Merged

Conversation

Deculsion
Copy link

No description provided.

@Deculsion Deculsion requested a review from cyanoei November 5, 2019 05:24
@@ -39,6 +39,7 @@ public CommandDictionary() {
commandDict.add(new Pair<>("list template", null));
commandDict.add(new Pair<>("list lost", null));
commandDict.add(new Pair<>("list person", null));
commandDict.add(new Pair<>("list person", "<MatricNo>"));
Copy link

@cyanoei cyanoei Nov 5, 2019

Choose a reason for hiding this comment

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

hmmmmmm. isn't this a form of list loan? to discuss irl.

@@ -45,16 +52,18 @@ private boolean sufficientStock() {
*/
public String execute(StockList list, Ui ui, Storage storage) {
String output = "";
if (sufficientStock()) {
if (!stockExists()) {
Copy link

Choose a reason for hiding this comment

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

nice

return new ListPersonCommand(CommandType.LIST);
case 2:
return new ListPersonLoansCommand(CommandType.LIST, inputArr[1]);
default:
Copy link

Choose a reason for hiding this comment

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

interesting implementation, but i'm not sure if we should change this to "list person all" and "list person [matric]" similar to the list stocktype issue. would be an easy change both ways tho

*/
public static int getLoanQuantity(String stockCode, String matricNo) {
public static int getPersonLoanQuantity(String stockCode, String matricNo) {
Copy link

Choose a reason for hiding this comment

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

good clarification

Copy link

@cyanoei cyanoei left a comment

Choose a reason for hiding this comment

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

nice

@cyanoei cyanoei merged commit 8909a07 into AY1920S1-CS2113T-F09-3:master Nov 5, 2019
cyanoei added a commit to cyanoei/eggventory-team that referenced this pull request Nov 5, 2019
@@ -69,5 +70,6 @@ public String execute(StockList list, Ui ui, Storage storage) throws BadInputExc
*/
public void execute(StockList list) {
list.addStock(stockType, stockCode, quantity, description);
LoanList.addStock(stockCode, quantity);
Copy link

Choose a reason for hiding this comment

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

so i realise that this is only implemented in one execute but not the other - theres two execute functions for normal adding versus for adding from storage, and this is only the storage one.

i will add this line into the other execute method as well in a future update.

Copy link

Choose a reason for hiding this comment

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

but also not sure if its more efficient to just add stocks to the loan's quantity list only when they are first ever loaned out? cos this current method requires the loanlist to quietly be updated when stocks are first added, which... doesn't quite feel necessary to me

@cyanoei cyanoei mentioned this pull request Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants