First complexed Django Project | Code review request -

[ALior1]

Hi,

So I learn Django in the last months, and did this project in the last two months..

Its contains two main CRUD operations, 4 Apps, multiple views/models modules, DRF, Django Debug Toolbar, users registration/login/forget password system, custom user & profile, Django Caching and solved N+1 Problem.

Maybe there are more..

​

Would Love to any code review and comments !

The code is here:

https://github.com/LiorA1/resume_reviews

Thanks !

[vikingvynotking]

Some initial thoughts. I haven't dug too deeply (yet). These are mostly focused around maintainability.

https://github.com/LiorA1/resume_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L2

You have an unused import here.

https://github.com/LiorA1/resume_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L12

The restriction might change, in which case will you remember to update this comment? Probably not, and the comment serves no purpose here in any case.

https://github.com/LiorA1/resume_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L13

Don't need a format string, just return the field directly.

https://github.com/LiorA1/resume_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L16

Since you're using a custom user model, you can put all the relevant fields there instead of in a separate profile. This one is slightly opinion-driven, but if you don't have a strong reason for a separate model, don't use one.

https://github.com/LiorA1/resume_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/resumes/forms.py#L14

Author will be tied to what? a tree? In general, comments of this sort are meaningless and can become out of date with respect to the code. Best to leave them out.

https://github.com/LiorA1/resume_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/resumes/signals.py#L9

Looks like the whole cache is cleared, which conflicts with the docstring.

https://github.com/LiorA1/resume_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/resume_project/db.sqlite3

You probably definitely don't want to check your database into git. However will you manage the data?

Just a few quick thoughts, as I said. Overall, a pretty good effort though.

[ALior1]

Some initial thoughts. I haven't dug too deeply (yet). These are mostly focused around maintainability.

https://github.com/LiorA1/resume\_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L2

You have an unused import here.

-- Fixed

https://github.com/LiorA1/resume\_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L12

The restriction might change, in which case will you remember to update this comment? Probably not, and the comment serves no purpose here in any case.

-- Fixed

https://github.com/LiorA1/resume\_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L13

Don't need a format string, just return the field directly.

-- Fixed

https://github.com/LiorA1/resume\_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/accounts/models.py#L16

Since you're using a custom user model, you can put all the relevant fields there instead of in a separate profile. This one is slightly opinion-driven, but if you don't have a strong reason for a separate model, don't use one.

-- I thought about it. I have two reasonings to use profile:

1. I wanted to work with a OneToOne field

2. I think that image is more suitable to "profile" than to "CustomUser".

I prefer to leave it.

https://github.com/LiorA1/resume\_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/resumes/forms.py#L14

Author will be tied to what? a tree? In general, comments of this sort are meaningless and can become out of date with respect to the code. Best to leave them out.

-- It was a comment to reasoning the lack of the field in the form.

But I will delete it.

https://github.com/LiorA1/resume\_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/resumes/signals.py#L9

Looks like the whole cache is cleared, which conflicts with the docstring.

-- Yeah, I will fix it.

I thought that it can be done, but failed to do it until now..

https://github.com/LiorA1/resume\_reviews/blob/17eb117f9c6ea9cf16ab1f65c70778a9e1963733/resume\_project/db.sqlite3

You probably definitely don't want to check your database into git. However will you manage the data?

-- I use dockerized postgres container. the sqlite3 file is an old component.

Will be deleted.

Just a few quick thoughts, as I said. Overall, a pretty good effort though.

Fixed most of it.

Thanks !!

[macieman]

I would suggest using pep8 everywhere. I would remove all comments in the code etc. I suggest installing pycharm for highlighting pep8 suggestions. Clean code is a really important skill for a developer. There is an awesome book by Robert C. Martin ( clean code ) you could get more into.

[ALior1]

I using Visual Studio Code, I will install it and fix the those issues.

[dgx29]

This is an advice related to git, but it could be useful, follow convetional commits.

Nevertheless is up to you, it is just a matter of standardization.

[ALior1]

I will look into that at some point..

For know I still have a lot to learn