Conversation
ysqty
left a comment
ysqty
left a comment
There was a problem hiding this comment.
Make the judgement call to SLAP whenever possible. It makes the code much more readable.
The process methods found in Controllers are rather long. Consider SLAP-ing them further if you can.
Try to insert the @@authortags before any header comments if there's any so that header comments can be seen in the collated file.
Try to standardize the names of constant variables whenever possible.
Replace all magic numbers with constants
Make sure header comments start with uppercase letter.
Only found a few indentation problems in the test cases.
AboutUs.md was not updated. Refer to this sample
*phew
Keep it up guys 👍
| # A0093907W | ||
| ###### /java/seedu/todo/controllers/AddController.java | ||
| ``` java | ||
| */ |
collated/main/A0093907W.md
Outdated
| } | ||
|
|
||
| @Override | ||
| public float inputConfidence(String input) { |
collated/main/A0093907W.md
Outdated
|
|
||
| @Override | ||
| public void process(String input) throws ParseException { | ||
|
|
collated/main/A0093907W.md
Outdated
| private LocalDateTime parseNatural(String natural) throws InvalidNaturalDateException { | ||
| Parser parser = new Parser(); | ||
| List<DateGroup> groups = parser.parse(natural); | ||
| Date date = null; |
There was a problem hiding this comment.
hmm can consider moving the first 2 lines into the try block. Usually codes in the try blocks are considered success cases
| } catch (InvalidNaturalDateException e) { | ||
| renderDisambiguation(isTask, name, naturalFrom, naturalTo); | ||
| return; | ||
| } |
| LocalDateTime testDateTime = fromEpoch(currTimeMs + 1); | ||
| Date actualDate = DateUtil.toDate(testDateTime); | ||
|
|
||
| assertNotEquals(testDate.getTime(), actualDate.getTime()); |
| LocalDateTime testDateTime1 = fromEpoch(testEpoch1); | ||
| LocalDateTime testDateTime2 = fromEpoch(testEpoch2); | ||
|
|
||
| assertEquals(DateUtil.floorDate(testDateTime1), DateUtil.floorDate(testDateTime2)); |
|
|
||
| @Test | ||
| public void floorDate_nullTest() { | ||
| assertEquals(DateUtil.floorDate(null), null); |
| } | ||
|
|
||
| private static LocalDateTime fromEpoch(long epoch) { | ||
| return LocalDateTime.ofInstant(Instant.ofEpochMilli(epoch), ZoneId.systemDefault()); |
collated/test/A0139812A.md
Outdated
| * TODO: Extract out method in AddController that can return task from command, | ||
| * and possibly remove the need to have taskToAdd. | ||
| */ | ||
| private void assertAddSuccess(String command, Task taskToAdd) { |
There was a problem hiding this comment.
good job in extracting out a util method here
louietyj
force-pushed
the
master
branch
3 times, most recently
from
2c0f07e to
ed011d9
Compare
Added menu items and key combinations for each view
CalendarItemFilter Part 1
CalendarItemFilter Part 2
Fix parseIsTaskEvent
Collated Codes
delete incorrect format collated codes
Sorry instead of delete the file, I wipe out the content only
Delete the file instead
Duplicated collated files
Fix UC21 - UC24 alignment
Update links for code written
Sorry Update wrongly. Just realise
Update links for code written
4 participants
Ready for Review