search

larpon / QtFirebase

An effort to bring Google's Firebase C++ API to Qt + QML
MIT License
284 stars 83 forks source link

Rewrite Admob code #63

Closed demiantres closed 5 years ago

demiantres commented 6 years ago

The Admob code is very ugly at the moment. We should rewrite it in the future.

larpon commented 6 years ago

It could very well be you're right - in order for others to validate you claims and be able to help - I suggest you maybe give some examples. Be a little more specific on where you think there's room for improvement :) Claiming something is ugly - is highly subjective - not everyone think the same things are pretty.

Bumping straight into a project and start pushing for rewrites, major changes and claim the code could be better is a bit useless without some sort of documentation to backup your claims. It's probably fine but we need to see what's wrong - and why we need to change it :)

Your contributions so far are very welcome, don't get me wrong - and I'm not against an AdMob rewrite - I just need to know why - so everyone can learn and discuss why things need to be done differently :)

demiantres commented 6 years ago

Just take a look at the code - it is a pain to make changes because there is no clear structure, comments are just copy&paste, ...

larpon commented 5 years ago

My point above is still valid. Show us how you'd like it to be - show us what a better structure is. Pull requests in this regard are welcome.

Closing for now, reopens are welcome when something constructive is ready.