nguyenngoc2505 / google_adwords

3 stars 0 forks source link

Model naming can be improved #6

Open olivierobert opened 7 years ago

olivierobert commented 7 years ago

The page model has 4 children models adword, bottom_adword, non_adword, top_adword but it's not obvious at first as these models are used.

Currently, these models are only used in the ExtractPage service with some meta-programming in create_page_variable. But due to the naming, it took me some time to understand the purpose of these models.

I recommend to rename these models as page_adword, page_bottom_adword, page_non_adword and page_top_adword. This small modification would help a lot in understanding the architecture of the model

(On another note, if these models are only used in this service and nowhere else, ExtractPage could be modified as a module with these models as inner classes. But it's hard to judge as these models might be used in other parts of the app if further developed)

nguyenngoc2505 commented 7 years ago

Thanks, I've fixed https://github.com/nguyenngoc2505/google_adwords/commit/d8942b05f1e076868e6374e35f37d4c6d62b0474