Merged
Conversation
# Conflicts: # README.md
# Conflicts: # README.md
…imien # Conflicts: # README.md
…I tried to add some style functionalities. It is not working yet.
# Conflicts: # README.md
So, I made a couple of changes: 1. I resolved the conflict between the implementation of the notes view from the remote repo and mine. Then, I deleted NoteView.java. 2. I deleted some classes: DBNoteDataAccessObject.java and DataAccessException.java. I did not need them since I am saving the notes permanently to a database. 3. I added an Input Data class and an Output Data class. I also added an InMemoryNoteDataAccessObject.java. 4. P.S. I am still implementing Clean Architecture.
…ation so that it takes Input Data from the controller at the beginning of the use case and gives Output Data to the presenter after executing the use case.
# Conflicts: # src/main/java/app/MainNoteApplication.java
Collaborator
|
Coding style looks good, I would recommend adding tests to make sure the code works as it's supposed to. Since we both created NoteInputData classes, we can combine them into one; I looked over your code to see where you create new NoteInputData instances and I think we should be able to update the class without much problem. The updated NoteInputData class would have methods getContent(), getTitle(), and getSelectedLanguage(). |
…rcus. I have cleaned up the code in NotesView by refactoring. I also adjusted the width of the program because of the notes view.
…rcus. I have cleaned up the code in NotesView by refactoring. I also adjusted the width of the program because of the notes view. Comment (on merged code from main): -Jenna, you cannot pass Note into any method in the Presenter. To update content of the note, you need to go view->controller->input boundary ->interactor->output boundary->presenter->view model->view. - You have to update the content of the note in the in-memory DAO. -Also, Presenter does not directly interact with View but with ViewModel. The Presenter cannot call a method in view. -Also, you cannot create an instance of Note in the View. -Also, you cannot pass Note into a method in the Controller.
lizakochel
approved these changes
Nov 28, 2024
Collaborator
lizakochel
left a comment
lizakochel
left a comment
There was a problem hiding this comment.
Coding style looks good, javadocs are there, doesn't seem to be any conflicts. I renamed by input and output data classes to include the word 'translation' so they don't get mixed up with yours.
lizakochel
reviewed
Nov 30, 2024
Collaborator
lizakochel
left a comment
lizakochel
left a comment
There was a problem hiding this comment.
- setTitle method in Note should return String but doesn't return anything
- In Note class, content and title are set as final variables so you can't change them in the setTitle and setContent methods
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is what I have done so far. I pushed it to my branch, but I want you guys to review it so it can be merged to main.