Open das7pad opened 6 years ago
yes following code should be removed, I added with a thought of if any new shortner implementation need to have some new attribute https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L49-L50
storing expanded
url because let say you shorten
a url and letter on in your code you want expanded
url so you don't need to do a network call
https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L68
same reason as above, you may need shorten
url and saving a network call
https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L88
I don't understand what's the problem here? https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L40
Even I don't get following, please elaborate what you are trying to convey here?
What were your thoughts to allow an actual derived BaseShortener
(let's call it Demo
) as the engine
argument for a Shortener
? One could simply create an instance of Demo
and use the api of it?
await asyncio.gather(shortener.short('https://github.com/'),
shortener.short('https://github.com/blikenoother/aiourlshortener/'),
return_exceptions=True)
shortener.shorten
can now store a short-url for 'https://github.com/'
or one for 'https://github.com/blikenoother/aiourlshortener/'
. It may also be None
in case both short
calls failed.
The point is that shortener.shorten
does not store a predictable content.
https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L40 It is a calculated value and the user knows the content of it as he used it for the init. This one can be refactored in a property
decorated function.
What were your thoughts to allow an actual derived
BaseShortener
(let's call itDemo
) as theengine
argument for aShortener
? One could simply create an instance ofDemo
and use the api of it?
We could refactor Shortener
into a function which returns a derived BaseShortener
instance.
There is no need to copy the public api of BaseShortener
.
gather
there will be issue, we don't need to set self.expanded
and self.shorten
We could refactor
Shortener
into a function which returns a derivedBaseShortener
instance. There is no need to copy the public api ofBaseShortener
.
can you pls explain this with sample code?
can you pls explain this with sample code?
https://github.com/das7pad/aiourlshortener/tree/refactor-shortener-class
These lines / instance attributes remain unused in the code:
the keyword arguments as attributes, however there are no defaults are set, access is dangerous. https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L49-L50 Let's drop the dynamic attribute access.
a possibly expanded url or user defined url to short https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L68 https://github.com/blikenoother/aiourlshortener/blob/1ea040e7abc583e6d52341490dd7b0e064b7c1da/aiourlshortener/shorteners/__init__.py#L100 Why do we store it?
a possibly shortened url or user defined url to expand https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L88 https://github.com/blikenoother/aiourlshortener/blob/1ea040e7abc583e6d52341490dd7b0e064b7c1da/aiourlshortener/shorteners/__init__.py#L75 See above.
The actual name of the derived
BaseShortener
: https://github.com/blikenoother/aiourlshortener/blob/fee8d34f5fa0e91579a5f1d8f61487894e8deb63/aiourlshortener/shorteners/__init__.py#L40And one more thing:
What were your thoughts to allow an actual derived
BaseShortener
(let's call itDemo
) as theengine
argument for aShortener
? One could simply create an instance ofDemo
and use the api of it?