search

Devsoc-BPGC / DoJMA

DoJMA news app for BITS Pilani K K Birla Goa Campus students
6 stars 18 forks source link

Added add video, update mess menu and add notification functionality to journalist app #155

Closed aryanagarwal15 closed 5 years ago

aryanagarwal15 commented 5 years ago

@Rushi98 @tanmaydixit please review and merge.

Rushi98 commented 5 years ago

@aryanagarwal15 the package naming convention based on type of the class (activity, adapter etc.) is not useful. We should instead base it on feature so that one gets clear idea how closely related classes interact and doesn't need to jump 4 packages to add / modify a feature.

Although the former package naming convention is used in the client app, we'll change it in next big refactor.

Also, package names in small case.

aryanagarwal15 commented 5 years ago

@Rushi98 MVC is universally accepted file format for professional projects. I really think that this frameworks makes more sense instead of one folder for one feature. Also moving through filesystem doesn't take many resources. So this makes more sense to me.

Rushi98 commented 5 years ago

@aryanagarwal15 MVC is a nice thing to have, but we aren't implementing it here. For ex- Our activity classes fetch data as well as manipulate UX.

Moving through file system requires mental context switches. It indeed consumes dev attention, unnecessarily.

aryanagarwal15 commented 5 years ago

@Rushi98 this update atleast implements some kind of file structure, which was not available before. It will be wasting resources redoing this rn. We can easily move forward with this structure without much/any problem in future. I for one have never seen profession projects using feature wise file structure breakup ever.

If you have someone else who can refactor this now, then by all means go forward, if not please merge this.

Rushi98 commented 5 years ago

continued in #157