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

Add command add product supplier #115

Conversation

Criss-Wang
Copy link

No description provided.

@codecov-io
Copy link

codecov-io commented Oct 11, 2020

Codecov Report

Merging #115 into master will increase coverage by 0.40%.
The diff coverage is 88.09%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #115      +/-   ##
============================================
+ Coverage     69.99%   70.39%   +0.40%     
- Complexity      551      563      +12     
============================================
  Files            89       91       +2     
  Lines          1853     1895      +42     
  Branches        217      222       +5     
============================================
+ Hits           1297     1334      +37     
- Misses          508      509       +1     
- Partials         48       52       +4     
Impacted Files Coverage Δ Complexity Δ
.../java/seedu/clinic/logic/commands/FindCommand.java 100.00% <ø> (ø) 10.00 <0.00> (ø)
...rc/main/java/seedu/clinic/model/attribute/Tag.java 80.00% <ø> (ø) 5.00 <0.00> (ø)
...seedu/clinic/logic/commands/AddProductCommand.java 85.18% <85.18%> (ø) 7.00 <7.00> (?)
...u/clinic/logic/parser/AddProductCommandParser.java 92.30% <92.30%> (ø) 4.00 <4.00> (?)
...n/java/seedu/clinic/logic/parser/ClinicParser.java 75.00% <100.00%> (+1.31%) 11.00 <0.00> (+1.00)
...c/main/java/seedu/clinic/model/attribute/Name.java 90.00% <100.00%> (ø) 7.00 <0.00> (ø)
.../java/seedu/clinic/storage/JsonAdaptedProduct.java 85.18% <0.00%> (+0.56%) 4.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4162055...5afbd9d. Read the comment docs.

@tohyuting tohyuting added this to the v1.2 milestone Oct 11, 2020
private final Product productToAdd;

/**
* Creates an AddCommand to add the specified {@code Supplier}

Choose a reason for hiding this comment

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

This should be AddProductCommand!

Choose a reason for hiding this comment

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

Will it be clearer if the statement is Creates an Addcommand to add the {@code Product} to the specified {@code Supplier}

@@ -9,8 +9,8 @@
*/
public class Tag {

public static final String MESSAGE_CONSTRAINTS = "Tags names should be alphanumeric";
public static final String VALIDATION_REGEX = "\\p{Alnum}+";
public static final String MESSAGE_CONSTRAINTS = "Tags names should be a word character";

Choose a reason for hiding this comment

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

I think this message could be clearer! Perhaps something along the lines of "Tag names should contain only 1 word"!

import seedu.clinic.model.supplier.Supplier;

/**
* Adds a product to a supplier to the CLI-nic app.

Choose a reason for hiding this comment

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

Perhaps this could be "Adds a product to a supplier in the CLI-nic app"!

}

public static Supplier getSupplierByName(Name supplierName, Model model) throws NoSuchElementException {
return model.getClinic().getSupplierList().stream()

Choose a reason for hiding this comment

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

Just wondering if this violates the law of Demeter?

Copy link

@zhengweii zhengweii left a comment

Choose a reason for hiding this comment

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

LGTM

@zhengweii zhengweii merged commit 44b53f0 into AY2021S1-CS2103-W14-4:master Oct 12, 2020
jeffreytjs pushed a commit to jeffreytjs/CLI-nic that referenced this pull request Oct 27, 2020
…nd-add-product-supplier

Add command add product supplier
jeffreytjs pushed a commit to jeffreytjs/CLI-nic that referenced this pull request Oct 27, 2020
…nd-add-product-supplier

Add command add product supplier
jeffreytjs pushed a commit to jeffreytjs/CLI-nic that referenced this pull request Oct 27, 2020
…nd-add-product-supplier

Add command add product supplier
jeffreytjs pushed a commit to jeffreytjs/CLI-nic that referenced this pull request Oct 27, 2020
…nd-add-product-supplier

Add command add product supplier
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.

None yet

4 participants