Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completed The Tasks!!! #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Ankur-Agrawal-ece20
Copy link

@Ankur-Agrawal-ece20 Ankur-Agrawal-ece20 commented Jul 19, 2021

CSoC Task 2 Submission

I have completed the following tasks

  • Stage 1
  • Stage 2
  • Stage 3
  • Stage 4

Copy link
Member

@m-e-l-u-h-a-n m-e-l-u-h-a-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work on the aassignment 🎉 . Your submission is evaluated.

Comment on lines +134 to +137
sum=0.0
for i in range(len(bookratings)):
sum=sum+bookratings[i].rating
book.rating=round((float)(sum/len(bookratings)), 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct but a more clean and Django-ish way for this could be (with looping):

book.rating = BookRating.objects.filter(book__pk=book_id).aggregate(Avg('rating'))['rating__avg']

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I'll make sure to use these functions from next time.

book = models.ForeignKey(Book, on_delete=models.CASCADE)
rater = models.CharField(max_length=122)
rated = models.BooleanField(default = False)
rating = models.SmallIntegerField(default=0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can also use MinValueValidator, `MaxValueValidator on this field to have database level rules for it.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I'll take care of this from now onwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants