Model definition #33

MariemBaccari · 2023-03-08T23:37:51Z

This PR aims to suggest a basis for the model definition by:

Creating the user class with the methods to build the user from its mandatory features.
Defining the Review class, MediaType enum as well as the Media superclass which all media types (such as the implemented Movie one) will extend.

An errorCheck package was also added to allow external handling of errors. A Preconditions class was added inside it and provides methods to check the validity of attributes. Many of them are commented with @todo and have to be completed during a later stage when the conditions are discussed.

Finally, some tests were added to test the User builder.

codeclimate · 2023-03-08T23:38:07Z

app/src/main/java/com/github/sdp/mediato/model/Review.java

+        this.media = media;
+    }
+
+    public Review(String username, Media media, int grade) {


Similar blocks of code found in 2 locations. Consider refactoring.

codeclimate · 2023-03-08T23:38:08Z

app/src/main/java/com/github/sdp/mediato/model/media/Media.java

+    private final String title;
+    private final String summary;
+
+    public Media(MediaType mediaType, String title, String summary) {


Similar blocks of code found in 2 locations. Consider refactoring.

This class should probably be abstract. No Media object should be instanciated, only subclasses of it.

simone-kalbermatter · 2023-03-09T08:42:05Z

app/src/main/java/com/github/sdp/mediato/model/media/Media.java

+    private final String title;
+    private final String summary;
+
+    public Media(MediaType mediaType, String title, String summary) {


This class should probably be abstract. No Media object should be instanciated, only subclasses of it.

simone-kalbermatter · 2023-03-09T08:58:17Z

app/src/main/java/com/github/sdp/mediato/model/User.java

+
+public class User {
+    public static final int LIMIT_LOCATION_SIZE = 2;
+    public static final String FAVOURITES_COLLECTION = "Favourites";


Just an idea: It could be good to have a class Collections and an an enum Collection type (such as Favourites, Recently Watched, Custom etc.). Then the user would have a List<Collection>. Like this we could still change the internal representation of a collection to being a List, Set etc... and define methods used on collections. I think it would also make it easier to later add more Collections. And in general it would be more clean to have the logic of what a collection is seperated from the User class.

I just committed changes where I suggest a similar implementation in [8b153b2]. I created the Collection class and CollectionType enum but left the Map<String, Collection> associated to the user since I think it can be useful for searching a collection by its name.

simone-kalbermatter · 2023-03-09T09:01:44Z

app/src/main/java/com/github/sdp/mediato/model/User.java

+    private String birthDate = "";
+    private String registerDate = "";
+
+    private List<Double> location = new ArrayList<>();


Is there a DB reason to define a location as a List<Double?

I initially wanted to use an array with fixed size but those are not accepted by Firebase. We could create a Location class but that sounds a bit overkill.

I think a Location class would be a useful abstraction to define such methods as getX() because from a list of double it is not obvious how to extract the coordinates for example.

Good point, adressed in commit [22e38d9].

simone-kalbermatter · 2023-03-09T09:02:33Z

app/src/main/java/com/github/sdp/mediato/model/User.java

+
+    private List<Double> location = new ArrayList<>();
+    private List<User> friends = new ArrayList<>();
+    private Map<String, List<Review>> reviews = new HashMap<>();


What would the String key value be here?

It would be the name of your collection "favourites", "highlights" ...

simone-kalbermatter · 2023-03-09T09:11:09Z

app/src/main/java/com/github/sdp/mediato/model/User.java

+        private List<User> friends = new ArrayList<>();
+        private Map<String, List<Review>> reviews = new HashMap<>();
+
+        public UserBuilder(String id){


Does this mean that only the String id is a required field to build a user? I think there should be more such as username and email.

All the fields except friends and reviews are "mandatory" (checked before the build() completes). I chose this model since it allows us to set the different fields from different fragments if we need to.

simone-kalbermatter · 2023-03-09T09:14:16Z

app/src/main/java/com/github/sdp/mediato/model/User.java

+    private List<User> friends = new ArrayList<>();
+    private Map<String, List<Review>> reviews = new HashMap<>();
+    private User(){}
+    public User(UserBuilder builder){


I think this constructor should be private if we are using the Builder design pattern.

simone-kalbermatter · 2023-03-09T09:24:07Z

app/src/test/java/com/github/sdp/mediato/modelTests.java

+    }
+
+    @Test
+    public void user_builder_fails_with_invalid_strings(){


Could you add a comment explaining what this test does? I'm not sure if this test is supposed to fail because there are missing attributes or because some strings are invalid.
We should also add tests that check that Preconditions fail on invalid strings but I guess that can wait for now as we haven't defined the formats yet.

Thanks for pointing that out :) I clarified its name in commit [2d89969].
Yes the rest of the tests should be added when we clearly define the necessary formats (that's valid for all the model classes and their attributes, this is just a "draft" so that the methods can be used by the rest of the team during the sprints).

MeKHell · 2023-03-09T07:23:50Z

app/src/main/java/com/github/sdp/mediato/model/media/Media.java

+    private final MediaType mediaType;
+    private final String title;
+    private final String summary;
+
+    public Media(MediaType mediaType, String title, String summary) {


Should not the Media also have a String property being the link to the displayed image?

Thanks for specifying that! Comment adressed in commit [53d58fc].

MeKHell · 2023-03-09T07:25:50Z

app/src/main/java/com/github/sdp/mediato/model/media/Movie.java

+public class Movie extends Media{
+    public Movie(MediaType mediaType, String title, String summary) {


Same comment : Movies will be displayed with a picture that is linked through an url.

Thanks ! Comment adressed in commit [53d58fc].

codeclimate · 2023-03-09T21:28:12Z

app/src/main/java/com/github/sdp/mediato/model/Review.java

+        this.grade = grade;
+    }
+
+    public Review(String username, Media media, int grade, String comment) {


Similar blocks of code found in 2 locations. Consider refactoring.

codeclimate · 2023-03-09T21:28:12Z

app/src/main/java/com/github/sdp/mediato/model/media/Media.java

+    private final String imageUrl;
+
+
+    public Media(MediaType mediaType, String title, String summary, String imageUrl) {


Similar blocks of code found in 2 locations. Consider refactoring.

MariemBaccari · 2023-03-10T08:21:10Z

The last two commits remove the birthdate and displayed name attribute from the user class which, after discussing with team members, is unecessary.

simone-kalbermatter

Looks good to me, the structure seems clean!

codeclimate · 2023-03-10T09:44:14Z

Code Climate has analyzed commit a25a809 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Duplication	2

The test coverage on the diff in this pull request is 39.2% (80% is the threshold).

This pull request will bring the total coverage in the repository to 40.7% (-3.5% change).

View more on Code Climate.

Adressed both comments

MariemBaccari added 3 commits March 8, 2023 22:53

add model definition for users, reviews and media

7e9591a

add preconditions

ae4df91

Add user builder tests

c1d82ac

MariemBaccari added the model Defining or modifying the model label Mar 8, 2023

codeclimate bot reviewed Mar 8, 2023

View reviewed changes

MariemBaccari requested review from markh0rr, MeKHell, PH-14, sgueissa and simone-kalbermatter March 8, 2023 23:38

MariemBaccari marked this pull request as draft March 8, 2023 23:40

fix code climate feedback

43a55c9

MeKHell assigned MariemBaccari Mar 9, 2023

MariemBaccari added 2 commits March 9, 2023 08:42

fix codeclimate threshold

0877a6c

fix codeclimate revert fix

fcf9ae7

MariemBaccari marked this pull request as ready for review March 9, 2023 08:16

simone-kalbermatter suggested changes Mar 9, 2023

View reviewed changes

MeKHell previously requested changes Mar 9, 2023

View reviewed changes

Make Media abstract and add String imageUrl attribute to Media

53d58fc

codeclimate bot reviewed Mar 9, 2023

View reviewed changes

MariemBaccari added 5 commits March 9, 2023 22:37

Adress Simone's comment by adding a Location class

22e38d9

Adresses Simone's comments about the user builder and its tests

2d89969

Add Collection class and CollectionType enum

8b153b2

Add untracked files

c13040e

Remove birthdate attribute from User class

23d4c6c

MariemBaccari added 4 commits March 10, 2023 09:37

remove displayed name attribute from User class

def6686

Add untracked file

0364cd3

fix typo in custom constructor

b8cfec7

Adresses Simone's IRL comment about Movie constructor

a25a809

simone-kalbermatter approved these changes Mar 10, 2023

View reviewed changes

MariemBaccari merged commit e11306a into main Mar 10, 2023

simone-kalbermatter removed request for sgueissa, PH-14 and markh0rr March 12, 2023 19:08

MariemBaccari mentioned this pull request Mar 14, 2023

Implement interface with Database #10

Closed

MariemBaccari deleted the model_definition branch March 20, 2023 17:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Model definition #33

Model definition #33

MariemBaccari commented Mar 8, 2023

codeclimate bot Mar 8, 2023

codeclimate bot Mar 8, 2023

simone-kalbermatter Mar 9, 2023

simone-kalbermatter Mar 9, 2023

simone-kalbermatter Mar 9, 2023 •

edited

Loading

MariemBaccari Mar 9, 2023

simone-kalbermatter Mar 9, 2023

MariemBaccari Mar 9, 2023

simone-kalbermatter Mar 9, 2023

MariemBaccari Mar 9, 2023

simone-kalbermatter Mar 9, 2023

MariemBaccari Mar 9, 2023

simone-kalbermatter Mar 9, 2023

MariemBaccari Mar 9, 2023

simone-kalbermatter Mar 9, 2023

simone-kalbermatter Mar 9, 2023

MariemBaccari Mar 9, 2023

MeKHell Mar 9, 2023

MariemBaccari Mar 9, 2023

MeKHell Mar 9, 2023

MariemBaccari Mar 9, 2023

codeclimate bot Mar 9, 2023

codeclimate bot Mar 9, 2023

MariemBaccari commented Mar 10, 2023 •

edited

Loading

simone-kalbermatter left a comment •

edited

Loading

codeclimate bot commented Mar 10, 2023

		public class Movie extends Media{
		public Movie(MediaType mediaType, String title, String summary) {

		private final String imageUrl;


		public Media(MediaType mediaType, String title, String summary, String imageUrl) {

Model definition #33

Model definition #33

Conversation

MariemBaccari commented Mar 8, 2023

codeclimate bot Mar 8, 2023

Choose a reason for hiding this comment

codeclimate bot Mar 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simone-kalbermatter Mar 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot Mar 9, 2023

Choose a reason for hiding this comment

codeclimate bot Mar 9, 2023

Choose a reason for hiding this comment

MariemBaccari commented Mar 10, 2023 • edited Loading

simone-kalbermatter left a comment • edited Loading

Choose a reason for hiding this comment

codeclimate bot commented Mar 10, 2023

simone-kalbermatter Mar 9, 2023 •

edited

Loading

MariemBaccari commented Mar 10, 2023 •

edited

Loading

simone-kalbermatter left a comment •

edited

Loading