I'm doing a Java software engineering team project in school and my team has decided on a Finance Tracker application, which contains the main components of Expense, Budget, Statistics, Suggestions and GUIPanels. Currently, the application only allows actions(add, delete, edit) on an expense to revolve around a primary budget and the statistics will also be tied to this primary budget. This is inspired from the git branching model that is available in the command line.
I’m in charge of the Statistics component and I am seeking to get some advice on Code Quality I can improve on. These are some of the classes I am interested to get advice for. Basically these set of classes are supposed to process information given by user input and translate it into Trend Lines(The Java files containing the main JavaFX code is not included here) The reason for possibly a weird implementation especially in the StatsTrendCommand is because I want to allow for optional parameters, so the constructors have to accept a null, and all other classes has to support this design consideration. Some other common concerns include:
1)How to better name the variables in the TrendStatistics class.
Variable of type double include periodicTotalExpenditure
Variables of 1 dimension List type include categoricalExpenses, periodicBudgetLimits, periodicTotalExpenditures, periodicCategoricalExpenditure
Variables of 2 dimension List include periodicCategorisedExpenses
2)What is a nice way to resolve the pass block in TrendStatistics class when both elements is required?
TrendStatistics.java
package seedu.address.model.statistics;
import static java.util.Objects.requireNonNull;
import java.util.ArrayList;
import java.util.List;
import javafx.collections.ObservableList;
import seedu.address.logic.commands.statistics.StatsTrendCommand;
import seedu.address.model.budget.Budget;
import seedu.address.model.budget.BudgetPeriod;
import seedu.address.model.category.Category;
import seedu.address.model.expense.Expense;
import seedu.address.model.expense.Timestamp;
/**
* Represents the Statistics class that provides a trend line as its Visual Representation method
*/
public class TrendStatistics extends Statistics {
public static final int INTERVAL_COUNT = 34;
private Timestamp startDate;
private Timestamp endDate;
private Budget primaryBudget;
private boolean budgetLimitMode;
private ObservableList<Expense> expenses;
private List<Timestamp> dates = new ArrayList<>();
private List<Double> periodicTotalExpenditures = new ArrayList<>();
private List<Double> periodicBudgetLimits = new ArrayList<>();
private List<ArrayList<Double>> periodicCategoricalExpenses = new ArrayList<>();
private TrendStatistics(ObservableList<Expense> expenses, List<Category> validCategories,
Timestamp startDate, Timestamp endDate, Budget primaryBudget, boolean isBudgetMode) {
super(expenses, validCategories);
this.startDate = startDate;
this.endDate = endDate;
this.expenses = getExpenses();
this.primaryBudget = primaryBudget;
this.budgetLimitMode = isBudgetMode;
if (!budgetLimitMode) {
for (int i = 0; i < validCategories.size(); i++) {
periodicCategoricalExpenses.add(new ArrayList<>());
}
}
}
/**
* Creates 2 trend lines to provide visual aid on the occurrence of total expenses relative to the budget limit
*
* @param expenses List of expenses
* @param validCategories List of allowed categories in MooLah
* @param startDate The start date of the tracking period
* @param endDate The end date of the tracking period
* @param primaryBudget The primary budget whose statistics is taken
*/
public static TrendStatistics run(ObservableList<Expense> expenses, List<Category> validCategories,
Timestamp startDate, Timestamp endDate,
Budget primaryBudget, boolean isBudgetMode) {
requireNonNull(primaryBudget);
boolean isStartPresent = startDate != null;
boolean isEndPresent = endDate != null;
if (isStartPresent && isEndPresent) {
//pass
} else if (isStartPresent) {
endDate = startDate.createForwardTimestamp(primaryBudget.getPeriod(),
2 * StatsTrendCommand.HALF_OF_PERIOD_NUMBER);
} else if (isEndPresent) {
startDate = endDate.createBackwardTimestamp(primaryBudget.getPeriod(),
2 * StatsTrendCommand.HALF_OF_PERIOD_NUMBER);
} else {
Timestamp centreDate = primaryBudget.getStartDate();
endDate = centreDate.createForwardTimestamp(primaryBudget.getPeriod(),
StatsTrendCommand.HALF_OF_PERIOD_NUMBER);
startDate = centreDate.createBackwardTimestamp(primaryBudget.getPeriod(),
StatsTrendCommand.HALF_OF_PERIOD_NUMBER);
}
TrendStatistics statistics = TrendStatistics.verify(expenses, validCategories,
startDate, endDate, primaryBudget, isBudgetMode);
statistics.generateTrendLine();
return statistics;
}
/**
* A method to practise defensive programming
* @param expenses List of expenses
* @param validCategories List of allowed categories in MooLah
* @param startDate The start date of the tracking period
* @param endDate The end date of the tracking period
* @param primaryBudget The primary budget whose statistics is taken
* @param isBudgetMode The condition to determine which mode is used
*/
private static TrendStatistics verify(ObservableList<Expense> expenses, List<Category> validCategories,
Timestamp startDate, Timestamp endDate,
Budget primaryBudget, boolean isBudgetMode) {
requireNonNull(startDate);
requireNonNull(endDate);
requireNonNull(expenses);
requireNonNull(validCategories);
return new TrendStatistics(expenses, validCategories,
startDate, endDate, primaryBudget, isBudgetMode);
}
/**
* Gathers the data to be used for the elements of the trend line
*/
private void generateTrendLine() {
ArrayList<ArrayList<ArrayList<Expense>>> data = new ArrayList<>();
BudgetPeriod period = primaryBudget.getPeriod();
Timestamp windowStartDate = primaryBudget.getStartDate();
Timestamp validDate = findClosestWindowStartDate(startDate, windowStartDate, period);
int intervalCount = 0;
while (hasInterval(validDate, endDate, period) && intervalCount < INTERVAL_COUNT) {
Timestamp localStartDate = validDate;
Timestamp nextLocalStartDate = localStartDate.plus(period.getPeriod());
Timestamp localEndDate = nextLocalStartDate.minusDays(1);
ArrayList<ArrayList<Expense>> periodicCategorisedExpenses =
getPeriodicCategorisedExpenses(localStartDate, localEndDate);
if (budgetLimitMode) {
double periodicTotalExpenditure = getTotalExpenditure(periodicCategorisedExpenses);
this.periodicTotalExpenditures.add(periodicTotalExpenditure);
periodicBudgetLimits.add(primaryBudget.getAmount().getAsDouble());
} else {
List<Double> periodicCategoricalExpenditure = getCategoricalExpenditure(periodicCategorisedExpenses);
flatMapAdd(periodicCategoricalExpenditure);
}
dates.add(localStartDate);
intervalCount++;
validDate = nextLocalStartDate;
}
this.setTitle(String.format("Periodic trendline from %s to %s in the unit of %ss",
startDate.showDate(), endDate.showDate(),
period));
}
/**
* Finds the window start date that is closest to the start date of interest
* @param startDate The start date for the trend line
* @param windowStartDate The start date for the current winndow
* @param period The duration of the window
*/
private Timestamp findClosestWindowStartDate(Timestamp startDate, Timestamp windowStartDate, BudgetPeriod period) {
Timestamp changedWindowDate = windowStartDate.createBackwardTimestamp(period);
while (windowStartDate.dateIsAfter(startDate) && changedWindowDate.dateIsAfter(startDate)) {
windowStartDate = changedWindowDate;
changedWindowDate = changedWindowDate.createBackwardTimestamp(period);
}
return windowStartDate;
}
/**
* Adds the periodic expenditure from each category to its respective category list
* @param periodicCategoricalExpenditure A list of total expenditure for each category
*/
private void flatMapAdd(List<Double> periodicCategoricalExpenditure) {
for (int i = 0; i < periodicCategoricalExpenditure.size(); i++) {
ArrayList<Double> categoricalExpenses = periodicCategoricalExpenses.get(i);
categoricalExpenses.add(periodicCategoricalExpenditure.get(i));
}
}
private ArrayList<ArrayList<Expense>> getPeriodicCategorisedExpenses(Timestamp startDate, Timestamp endDate) {
TabularStatistics statistics = new TabularStatistics(expenses, getValidCategories(), startDate, endDate);
ArrayList<ArrayList<Expense>> dataWithTotal = statistics.extractRelevantExpenses(startDate, endDate);
dataWithTotal.remove(dataWithTotal.size() - 1);
return dataWithTotal;
}
private double getExpenditureForCategory(ArrayList<Expense> categorisedExpenses) {
double total = 0;
for (Expense expense : categorisedExpenses) {
total += Double.parseDouble(expense.getPrice().value);
}
return total;
}
private double getTotalExpenditure(ArrayList<ArrayList<Expense>> data) {
double total = 0;
for (ArrayList<Expense> categorisedExpenses : data) {
total += getExpenditureForCategory(categorisedExpenses);
}
return total;
}
private List<Double> getCategoricalExpenditure(ArrayList<ArrayList<Expense>> data) {
ArrayList<Double> result = new ArrayList<>();
for (ArrayList<Expense> categorisedExpenses : data) {
result.add(getExpenditureForCategory(categorisedExpenses));
}
return result;
}
public List<ArrayList<Double>> getPeriodicCategoricalExpenses() {
return periodicCategoricalExpenses;
}
private static boolean hasInterval (Timestamp validDate, Timestamp endDate, BudgetPeriod period) {
return validDate.isBefore(endDate) && (validDate.plus(period.getPeriod())).isBefore(endDate);
}
public List<Timestamp> getDates() {
return dates;
}
public List<Double> getPeriodicTotalExpenditure() {
return periodicTotalExpenditures;
}
public List<Double> getPeriodicBudgetLimits() {
return periodicBudgetLimits;
}
public boolean isBudgetLimitMode() {
return budgetLimitMode;
}
public String toString () {
return String.format("%s\n%s", getTitle(), getPeriodicTotalExpenditure());
}
}
Statistics.java
package seedu.address.model.statistics;
import static java.util.Objects.requireNonNull;
import java.util.List;
import javafx.collections.ObservableList;
import seedu.address.logic.commands.statistics.StatsCommand;
import seedu.address.logic.commands.statistics.StatsCompareCommand;
import seedu.address.logic.commands.statistics.StatsTrendCommand;
import seedu.address.model.budget.Budget;
import seedu.address.model.category.Category;
import seedu.address.model.expense.Expense;
import seedu.address.model.expense.Timestamp;
/**
* Represents the Statistics class in MooLah.
*/
public class Statistics {
public static final String MESSAGE_CONSTRAINTS_END_DATE = "Start date must be before end date.";
private ObservableList<Expense> expenses;
private final List<Category> validCategories;
private int categorySize;
private String title;
/**
* Creates a Statistics object
* @param expenses A list of expenses in the current budget
* @param validCategories A list of tags used among all expenses
*/
public Statistics(ObservableList<Expense> expenses, List<Category> validCategories) {
requireNonNull(validCategories);
requireNonNull(expenses);
this.expenses = expenses;
this.validCategories = validCategories;
this.categorySize = validCategories.size();
}
/**
* Returns the lists of all expenses in the current budget
*/
public ObservableList<Expense> getExpenses() {
return expenses;
}
public String getTitle() {
return title;
}
List<Category> getValidCategories() {
return validCategories;
}
int getCategorySize() {
return categorySize;
}
/**
* The main handler method of the Statistics model object to identify what kind of Statistics has to be done
* with each command word
* @param expenses List of expenses
* @param command Command word provided by the user
* @param date1 First date input given by the user
* @param date2 Second date input given by the user
* @param primaryBudget The primary budget whose statistics is taken
*/
public static Statistics calculateStats(ObservableList<Expense> expenses, String command,
Timestamp date1, Timestamp date2,
Budget primaryBudget, boolean isBudgetMode) {
requireNonNull(expenses);
List<Category> validCategories = Category.getValidCategories();
switch (command) {
case StatsCommand.COMMAND_WORD:
return PieChartStatistics.run(expenses, validCategories, date1, date2, primaryBudget);
case StatsCompareCommand.COMMAND_WORD:
return TabularStatistics.run(expenses, validCategories, date1, date2, primaryBudget);
case StatsTrendCommand.COMMAND_WORD:
return TrendStatistics.run(expenses, validCategories, date1, date2, primaryBudget, isBudgetMode);
default:
return null;
}
}
void setTitle(String title) {
this.title = title;
}
}
StatsTrendCommand.java
package seedu.address.logic.commands.statistics;
import static java.util.Objects.requireNonNull;
import static seedu.address.commons.core.Messages.MESSAGE_DISPLAY_STATISTICS_WITHOUT_BUDGET;
import static seedu.address.logic.parser.CliSyntax.PREFIX_END_DATE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_MODE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_START_DATE;
import seedu.address.logic.commands.Command;
import seedu.address.logic.commands.CommandResult;
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.Model;
import seedu.address.model.expense.Timestamp;
import seedu.address.model.statistics.Mode;
import seedu.address.model.statistics.TrendStatistics;
import seedu.address.ui.StatsPanel;
/**
* Calculates and displays statistics
*/
public class StatsTrendCommand extends Command {
public static final String COMMAND_WORD = "statstrend";
public static final String MESSAGE_USAGE = COMMAND_WORD
+ ": Shows statistics trends for regular periods between the Start Date and End Date. "
+ "Parameters: "
+ PREFIX_START_DATE + "START_DATE "
+ PREFIX_END_DATE + "END_DATE "
+ PREFIX_MODE + "CATEGORY_OR_BUDGET "
+ "\nExample: " + COMMAND_WORD + " "
+ PREFIX_START_DATE + "11-11-1111 "
+ PREFIX_END_DATE + "12-12-1212 "
+ PREFIX_MODE + "category";
public static final String MESSAGE_SUCCESS = "Statistics Trend Calculated!";
public static final int HALF_OF_PERIOD_NUMBER = TrendStatistics.INTERVAL_COUNT / 2;
private final Timestamp startDate;
private final Timestamp endDate;
private final boolean mode;
private StatsTrendCommand(Timestamp date1, Timestamp date2, Mode mode) {
requireNonNull(mode);
this.startDate = date1;
this.endDate = date2;
this.mode = mode.isBudgetMode();
}
@Override
protected void validate(Model model) throws CommandException {
requireNonNull(model);
if (!model.hasPrimaryBudget()) {
throw new CommandException(MESSAGE_DISPLAY_STATISTICS_WITHOUT_BUDGET);
}
}
@Override
public CommandResult execute(Model model) {
requireNonNull(model);
model.calculateStatistics(COMMAND_WORD , startDate, endDate, mode);
return new CommandResult(MESSAGE_SUCCESS, false, false, StatsPanel.PANEL_NAME);
}
/**
* Creates a StatsTrendCommand that only contains a start date
* @param startDate The start date
* @param mode The mode specified by the user
*/
public static StatsTrendCommand createOnlyWithStartDate(Timestamp startDate, Mode mode) {
requireNonNull(startDate);
return new StatsTrendCommand(startDate, null, mode);
}
/**
* Creates a StatsTrendCommand that only contains an end date
* @param endDate The end date
* @param mode The mode specified by the user
*/
public static StatsTrendCommand createOnlyWithEndDate(Timestamp endDate, Mode mode) {
requireNonNull(endDate);
return new StatsTrendCommand(null, endDate, mode);
}
/**
* Creates a StatsTrendCommand that contains a start date and an end date
* @param startDate The start date
* @param endDate The end date
* @param mode The mode specified by the user
*/
public static StatsTrendCommand createWithBothDates(Timestamp startDate, Timestamp endDate, Mode mode) {
requireNonNull(startDate);
requireNonNull(endDate);
requireNonNull(mode);
return new StatsTrendCommand(startDate, endDate, mode);
}
/**
* Creates a StatsTrendCommand that does not contain a start date or end date
* @param mode The mode specified by the user
*/
public static StatsTrendCommand createWithNoDate(Mode mode) {
return new StatsTrendCommand(null, null, mode);
}
@Override
public boolean equals(Object other) {
return other == this //short circuit if same object
|| (other instanceof StatsTrendCommand // instance of handles nulls
&& startDate.equals(((StatsTrendCommand) other).startDate)
&& endDate.equals(((StatsTrendCommand) other).endDate));
}
}
StatsTrendCommandParser.java
package seedu.address.logic.parser;
import static seedu.address.commons.core.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.commons.core.Messages.MESSAGE_REPEATED_PREFIX_COMMAND;
import static seedu.address.logic.parser.CliSyntax.PREFIX_END_DATE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_MODE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_START_DATE;
import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;
import seedu.address.logic.commands.statistics.StatsTrendCommand;
import seedu.address.logic.parser.exceptions.ParseException;
import seedu.address.model.expense.Timestamp;
import seedu.address.model.statistics.Mode;
import seedu.address.model.statistics.Statistics;
/**
* Parses input arguments and creates a new StatsTrendCommand object
*/
public class StatsTrendCommandParser implements Parser<StatsTrendCommand> {
public static final List<Prefix> REQUIRED_PREFIXES = Collections.unmodifiableList(List.of(PREFIX_MODE));
public static final List<Prefix> OPTIONAL_PREFIXES = Collections.unmodifiableList(List.of(
PREFIX_START_DATE, PREFIX_END_DATE));
/**
* Parses the given {@code String} of arguments in the context of the StatsTrendCommand
* and returns an StatsTrendCommand object for execution.
* @throws ParseException if the user input does not conform the expected format
*/
public StatsTrendCommand parse(String args) throws ParseException {
ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_START_DATE, PREFIX_END_DATE, PREFIX_MODE);
if (!arePrefixesPresent(argMultimap, PREFIX_MODE) || !argMultimap.getPreamble().isEmpty()) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, StatsTrendCommand.MESSAGE_USAGE));
}
if (hasRepeatedPrefixes(argMultimap, PREFIX_START_DATE, PREFIX_END_DATE, PREFIX_MODE)) {
throw new ParseException(MESSAGE_REPEATED_PREFIX_COMMAND);
}
Timestamp startDate = null;
Timestamp endDate = null;
boolean isStartPresent = argMultimap.getValue(PREFIX_START_DATE).isPresent();
boolean isEndPresent = argMultimap.getValue(PREFIX_END_DATE).isPresent();
Mode mode = ParserUtil.parseMode(argMultimap.getValue(PREFIX_MODE).get());
if (isStartPresent && isEndPresent) {
checkStartBeforeEnd(argMultimap);
startDate = ParserUtil.parseTimestamp(argMultimap.getValue(PREFIX_START_DATE).get());
endDate = ParserUtil.parseTimestamp(argMultimap.getValue(PREFIX_END_DATE).get());
return StatsTrendCommand.createWithBothDates(startDate, endDate, mode);
} else if (isStartPresent) {
startDate = ParserUtil.parseTimestamp(argMultimap.getValue(PREFIX_START_DATE).get());
return StatsTrendCommand.createOnlyWithStartDate(startDate, mode);
} else if (isEndPresent) {
endDate = ParserUtil.parseTimestamp(argMultimap.getValue(PREFIX_END_DATE).get());
return StatsTrendCommand.createOnlyWithEndDate(endDate, mode);
} else {
return StatsTrendCommand.createWithNoDate(mode);
}
}
/**
* Returns true if none of the prefixes contains empty {@code Optional} values in the given
* {@code ArgumentMultimap}.
*/
private static boolean arePrefixesPresent(ArgumentMultimap argumentMultimap, Prefix... prefixes) {
return Stream.of(prefixes).allMatch(prefix -> argumentMultimap.getValue(prefix).isPresent());
}
/**
* Returns true at least one prefix have more than to one value
* {@code ArgumentMultiMap}.
*/
private static boolean hasRepeatedPrefixes(ArgumentMultimap argumentMultimap, Prefix... prefixes) {
return !(Stream.of(prefixes).allMatch(prefix -> argumentMultimap.getAllValues(prefix).size() <= 1));
}
/**
* Parses the given {@code String} of arguments in the context of the StatsTrendCommand
* Checks that start date is before the end date of the given {@code ArgumentMultimap}
*
* @throws ParseException if the detected start date is after the end date
*/
private void checkStartBeforeEnd(ArgumentMultimap argMultimap) throws ParseException {
Timestamp startDate = ParserUtil.parseTimestamp(argMultimap.getValue(PREFIX_START_DATE).get());
Timestamp endDate = ParserUtil.parseTimestamp(argMultimap.getValue(PREFIX_END_DATE).get());
if (endDate.isBefore(startDate)) {
throw new ParseException(Statistics.MESSAGE_CONSTRAINTS_END_DATE);
}
}
}
Relevant section of ModelManager.java
@Override
public void calculateStatistics(String command, Timestamp date1, Timestamp date2, boolean isBudgetMode) {
ObservableList<Expense> primaryBudgetExpenses = getPrimaryBudget().getExpenses();
Statistics statistics = Statistics.calculateStats(primaryBudgetExpenses, command, date1, date2,
getPrimaryBudget(), isBudgetMode);
this.setStatistics(statistics);
}
public Statistics getStatistics() {
return statistics;
}
public void setStatistics(Statistics statistics) {
requireNonNull(statistics);
this.statistics = statistics;
}
I am open to any kind of feedback on code quality, and I’m still testing whether the code works logically. Therefore, I want the code here to be checked not for logic errors(unless there is some fatal flaw you observe) but instead for code quality. Do inform me if more code or some clarification is required for a better review. Thanks in advance.
Update
The code works now, completed the JUnit tests, whose code I won't be showing here until requested.
