Feature/add reels models final #188
Conversation
| public long MaxThumbnailsPerSprite { get; set; } | ||
| public long TotalThumbnailNumPerSprite { get; set; } | ||
|
|
||
| bool IEquatable<AnimatedThumbnail>.Equals(AnimatedThumbnail other) { |
There was a problem hiding this comment.
My impressions is that this check should consider fewer fields. For example, when comparing two thumbnails, one could come with a nulled/zeroed ThumbnailsPerRow, and then, even though all the other fields are the same, the program would identify them as being two different entities. I think ThumbnailsPerRow or MaxThumbnailsPerSprite is not an ontological attribute (omg how pedantic) of the AnimatedThumbnail. Also, regarding FileSizeKb, the encoding used by Instagram could change, compression level and other stuff, and this would result in them being different, when, for the end-users, they would be the same
|
|
||
| namespace DataLakeModels.Models.Reels { | ||
| public class Caption : IEquatable<Caption> { | ||
| public string Pk { get; set; } |
There was a problem hiding this comment.
What is Pk? Primary Key?
| namespace DataLakeModels.Models.Reels { | ||
| public class Caption : IEquatable<Caption> { | ||
| public string Pk { get; set; } | ||
| public string Text { get; set; } |
There was a problem hiding this comment.
TEXT is not a reserved word in PSQL, and even then, quoted strings can always be used, even if they are reserved words. However, I would consider avoiding this, in order not to have any kind of confusion during development
| public string ReelId { get; set; } | ||
| public string Status { get; set; } | ||
| public string MediaId { get; set; } | ||
| public long BitFlags { get; set; } |
| ContentType == other.ContentType && | ||
| ShareEnabled == other.ShareEnabled && | ||
| CreatedAtUTC == other.CreatedAtUTC && | ||
| DidReportAsSpam == other.DidReportAsSpam && |
There was a problem hiding this comment.
I think this is not necessary for equality check
| public MashupInfo MashupInfo { get; set; } | ||
| public string ShoppingInfo { get; set; } | ||
| public string TemplateInfo { get; set; } | ||
| public string ChallengeInfo { get; set; } |
There was a problem hiding this comment.
Just taking note, this is also something that TikTok has
| public string ClipsCreationEntryPoint { get; set; } | ||
| public string ViewerInteractionSettings { get; set; } | ||
|
|
||
| bool IEquatable<ClipsMeta>.Equals(ClipsMeta other) { |
There was a problem hiding this comment.
Isn't the Id enough to guarantee equality? Given that it will be generated by the DB as identity
| public bool AllowCreatorToRename { get; set; } | ||
| public string ProgressiveDownloadUrl { get; set; } | ||
| public bool CanRemixBeSharedToFb { get; set; } | ||
| public long? FormattedClipsMediaCount { get; set; } |
There was a problem hiding this comment.
Why should this be nullable? Can we just set the count to zero if there is no information available?
| public string Code { get; set; } | ||
| public User User { get; set; } | ||
| public string UserId { get; set; } | ||
| public Caption Caption { get; set; } |
There was a problem hiding this comment.
Maybe we can keep all the entity relationships (foreign keys) at the top, for better readability
|
|
||
| public ICollection<ReelStats> Stats { get; set; } | ||
|
|
||
| bool IEquatable<Reel>.Equals(Reel other) { |
There was a problem hiding this comment.
I think this is too much information. Not only it makes hard to debug, but might hurt performance, if we need to check new entries against thousands of existing entries
No description provided.