Martialdelastic / google-api-adwords-php

Automatically exported from code.google.com/p/google-api-adwords-php
Apache License 2.0
0 stars 0 forks source link

Add support for class name prefix #4

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
This is not a bug report but more an call for improvement. 

The client is using classes like Criterion, Keyword etc.. In the 
application I'm writing I also use these names which results in classnames 
clashes. 

I'm changing my class names to ones with prefixes but some core Symfony 
libraries also uses the same class names (Like Criterion).

Namespaces are in PHP5.3 but for now maybe consider the use of class 
prefixes to prevent this type of clashes?

I am aware that this is a real major change to the code but maybe in the 
future please consider it :) 

Thanks

Original issue reported on code.google.com by leonrenk...@gmail.com on 10 Nov 2009 at 10:48

GoogleCodeExporter commented 8 years ago
Hi, thank you for bringing this to our attention.  We initially wanted to avoid 
class 
name prefixes, as many of the API classes already have very long names (like 
CampaignAdExtensionReturnValue).  Renaming your classes is the solution we had 
in 
mind, but I can see how this would be a problem when the conflict is with 
another 
library.  I'll take another look at this problem and see if there is a better 
solution.

Original comment by api.ekol...@gmail.com on 10 Nov 2009 at 3:41

GoogleCodeExporter commented 8 years ago
The best solution is to wait for PHP 5.3 and then use namespaces :) 

The solution i'm now working on is a 'proxy'-webservice which is a bridge 
between my 
application and your library. This could scale also because i'm working over 
http with 
json. 
So maybe it's an even better solution :)

Thanks for your reply and very thanks for all efforts you took in the library

Original comment by leonrenk...@gmail.com on 10 Nov 2009 at 3:48

GoogleCodeExporter commented 8 years ago

Original comment by api.ekol...@gmail.com on 19 Jan 2010 at 8:34

GoogleCodeExporter commented 8 years ago
I'd support the use of prefixes. 

A library that requires the application code to change class names of obvious 
objects
like "Campaign", seems a little awkward to implement. What if there is another
library that goes the same route?

And besides the fact that I don't really like the namespace implementation in 
PHP 5.3
I think it would be a mistake to change the Adwords client library any time 
soon to
use namespaces and hence require PHP 5.3 to run.

Long story short, I'd prefer to have a little longer class names over having to
adjust my application code to a third party library. 

@leonrenkema: I have recently used Gearman http://gearman.org/ quite a bit and 
find
that to be a good way to separate modules. It also allows to schedule jobs to 
be run
in parallel which is a nice to have. You might want to look into that too. It 
comes
with a persistent storage too, so you don't lose jobs if the client dies.

Original comment by volker.k...@gmail.com on 19 Jan 2010 at 8:46

GoogleCodeExporter commented 8 years ago
To require an application to changes it's core code to accommodate your 
library's class 
naming conventions seems absurd to me. For this reason, I can't use this 
library as 
expected within my own applications as it's unfeasable to change 100s of lines 
of code.

I'd vote this up as being a normal to high priority request, rather than low.

Thanks

Original comment by ncldavies on 26 Jan 2010 at 12:47

GoogleCodeExporter commented 8 years ago
I think the problem is that it isn't a minor change.. It's (almost) a complete 
rewrite of the whole code and it 
breaks compatibility with older versions.
What I have done for the time being is running the library on a different 
domain and acts like a webservice 
working on json. That way I can easily call the adwords client without the risk 
of namespace problems. 

Original comment by leonrenk...@gmail.com on 26 Jan 2010 at 12:55

GoogleCodeExporter commented 8 years ago
Sure, I understand the amount of work needed and the issues that would cause. 
By voting 
up, I suggest releasing a v2 ASAP.

The workarounds needed to accommodate this library for the majority of use 
cases are 
fairly ugly and/or unnecessary when this library should in essence be plug and 
play.

Original comment by ncldavies on 26 Jan 2010 at 1:35

GoogleCodeExporter commented 8 years ago
Changing planned version number.

Original comment by api.ekol...@gmail.com on 17 Feb 2010 at 3:52

GoogleCodeExporter commented 8 years ago

Original comment by api.ekol...@gmail.com on 8 Apr 2010 at 4:07

GoogleCodeExporter commented 8 years ago
Issue 49 has been merged into this issue.

Original comment by api.ekol...@gmail.com on 6 Dec 2010 at 10:06

GoogleCodeExporter commented 8 years ago
This feature has been added in r196 as an option at build time.  See the 
updated README for details on how to use this feature.

Original comment by ekoleda+devrel@googlers.com on 20 Jul 2011 at 4:00

GoogleCodeExporter commented 8 years ago
Our app is adding a 'Logger' class that is going to conflict with the AdWords 
Client Library 'Logger' class in <./Utils/Logger.php>. Perhaps these in 
particular need the `GoogleApiAdWords_` prefixed by default since they are 
usually not called by anything other than vendor routines (not used by the 
integrating developer). Alternatively, the custom build.xml process could be 
modified to also rewrite these as well?

Related: #49

Original comment by stephen....@gmail.com on 9 Dec 2011 at 1:57

GoogleCodeExporter commented 8 years ago
Reopening issue #49, as that is more to this point.

Original comment by ekoleda+devrel@googlers.com on 9 Dec 2011 at 2:47