Closed GoogleCodeExporter closed 8 years ago
Hi Corneliu,
The goal of a good API is to expose externally only those functionalities that
are required and hide as many implementation details as possible.
Making some constructors internal is one of our design choices.
Claudio
Original comment by ccherub...@google.com
on 18 Jun 2011 at 6:38
Claudio,
If you want to hide implementation details you make classes internal as
well. Making constructors only internal will only get users upset in not
being able to use them.
Also, you should only do that if you provide other extensibility ways.
Otherwise all you achieve is a closed and not-extensible API not fun to work
with.
For example GAuthSubRequestFactory does not have a logging version of it so
if I want to do some logging in the GAuthSubRequest I need to extend it.
Also, the current logging classes all assume I want a file which is not the
scenario for me for example as I need the logging to be consolidated with
other logging. In order to extend I had to do several changes:
1. Make GAuthSubRequestFactory constructor public - so I can extend the
class
2. Make GAuthSubRequest constructor public - so I can extend the class
3. Make responseStream in the GAuthSubRequest protected so I can "swap"
it after I read and log it (the streams can be read only once so if I want
to log I need to read in memory then swap it so the base class can still
work)
4. Make the RequestCopy stream in the GAuthSubRequest protected so I can
read it as well and reset it back to beginning so the request can proceed.
I don't mind changing the source code but maybe some other people do.
Hiding is good when there are build in extensibility points.
My 2 cents,
Corneliu
Original comment by corneliu...@gtempaccount.com
on 19 Jun 2011 at 5:53
Hi Corneliu,
Some of the design choices were made before I joined this team and I may have
missed some background but nothing is set in stone and a valid suggestion is
always accepted.
I see your point and I appreciate your contribution, would you like to submit a
patch to change the constructors' visibility as appropriate?
Thanks
Claudio
Original comment by ccherub...@google.com
on 20 Jun 2011 at 9:43
Hi Claudio,
I will prepare a patch once I do all the changes I want and it's all stable.
Original comment by corneliu...@gtempaccount.com
on 20 Jun 2011 at 11:34
Marking this as obsolete, feel free open a new entry to propose your patch.
Original comment by ccherub...@google.com
on 27 Dec 2011 at 4:18
Original issue reported on code.google.com by
corneliu...@gtempaccount.com
on 18 Jun 2011 at 1:43