Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The code polishing #11

Closed
sleptor opened this issue May 4, 2017 · 3 comments
Closed

The code polishing #11

sleptor opened this issue May 4, 2017 · 3 comments

Comments

@sleptor
Copy link

sleptor commented May 4, 2017

Let's make the code more beautiful and compact :)

  1. You extended \yii\db\Command. It is ok. But you copy-pasted a lot of unnecessary code from a parent class to your child class. You should clean it up.

for example, properties
public $db;
public $params = [];
...
and so on
must be removed from your class
some unnecessary methods should be removed as well

I checked only Command class, but I guess other classes may be affected as well.
Please fix it.

  1. https://github.com/sanchezzzhak/kak-clickhouse/blob/master/Command.php#L332
    I guess preg_match may be replaced with stripos here

  2. to be continued :)

sanchezzzhak added a commit that referenced this issue May 4, 2017
@sanchezzzhak
Copy link
Owner

Thanks

@sleptor
Copy link
Author

sleptor commented May 4, 2017

I guess a lot of methods of Command class can be removed as well
cache
noCache
getSql
setSql (why did you change it?)
queryAll
queryOne
bindValues (why schema checking was commented out? you set $typeMap in Scheme class, all has to work good)

maybe some other methods may be removed also..

the main idea is the MOST methods MUST be inherited from a parent class

$_sql & $_pendingParams must be removed as well

@sanchezzzhak
Copy link
Owner

sanchezzzhak commented May 4, 2017

setSql (why did you change it?)

I do not remember (((

getSql, setSql, queryAll, queryOne - removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants