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

changes in calculator #162

Merged

Conversation

yuqiaoluolong
Copy link

Added a CalculatorData class to take in a FoodList and return information needed to calculate to Calculator class.

Added the initialization of CalculatorData class in manager.
Deleted setCalculator method in manager.

Changed the inputs of methods related with calculator in CalculateCommand class.

To update calculator now, need to:
data.inputData(foodList);
calculator.update(data);

yuqiaoluolong added 6 commits October 27, 2020 22:45
The calculator version2 brunch enables calculation of recommended calorie intake and food nutritions from food with time in a specific time range.
* 'master' of https://github.com/AY2021S1-CS2113-T14-4/tp: (82 commits)
  Update UserGuide.md
  Fix formatting
  Fix major formatting issues
  Update _config.yml
  Future date exception
  Add command with date time
  Update UserGuide.md
  minor fixes
  Add examples of usage to UserGuide section
  Put Examples of Usage for adding controling food items into the User Guide
  add addFoodAtDateTime
  Update help command message
  Bug Fixes
  Add case where gender is male
  Update ExitCommand.java
  Database Printing
  Feature Updates
  add toString version of datetime support methods
  Add dataSuccessfullySavedMessage method
  Delete Food.java in Test
  ...
…tion needed to calculate to Calculator class.

Added the initialization of CalculatorData class in manager.
Deleted setCalculator method in manager.

Changed the inputs of methods related with calculator in CalculateCommand class.
Copy link

@0xEljh 0xEljh left a comment

Choose a reason for hiding this comment

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

Recommending some logic changes regarding how portion sizes are handled.

use the getPortioned...() versions of the get methods of FoodList. e.g. getPortionedFoods() and getPortionedFoodsAfterDateTime( .. )

}

public List<Integer> getTotalCalorie() {
this.foods = list.getFoods();
Copy link

Choose a reason for hiding this comment

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

I suspect that the method called should be getPortionedFoods()

getFoods() does not take into consideration the portion size provided by the user and it seems that you did not get and handle the portion sizes separately. Using getPortionedFoods() will return a food item with scaled nutrional values (different from the item the user entered)

}

public List<Integer> getTotalCalorie(LocalDateTime startTime) {
this.foods = list.getFoodsAfterDateTime(startTime);
Copy link

Choose a reason for hiding this comment

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

Similar to the other comment. I suspect that this should be getPortionedFoodsAfterDateTime().

These apply throughout similar methods in this class.

foodList.add(new Food("bao", 290, 0, 16, 0));
Calculator calculator = new Calculator(foodList);
FoodList foodList = new FoodList();
foodList.addFood(1, new Food("chicken rice", 666, 55, 30, 0));
Copy link

Choose a reason for hiding this comment

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

Consider adding tests with portion size >1. This would have caught the potential issue raised in earlier comments about portion sizing.

Copy link
Author

Choose a reason for hiding this comment

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

OK, I changed the corresponding methods.

Copy link

@0xEljh 0xEljh left a comment

Choose a reason for hiding this comment

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

LGTM now

@yuqiaoluolong yuqiaoluolong merged commit 539b522 into AY2021S1-CS2113-T14-4:master Nov 7, 2020
@tikimonarch tikimonarch linked an issue Nov 9, 2020 that may be closed by this pull request
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.

[PE-D] Calculating Carbohydrate on Empty List Still Yields Result
2 participants