Is there a problem using a “shared” private property in a class to build fluent/evolving DB query?
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty{ height:90px;width:728px;box-sizing:border-box;
}
0
I'm trying to write a class that relies on some legacy code to fetch data from database related to a particular table (productcode)
For not repeating myself, I implement a private property $sql that holds the "evolving" query and finally applying getResult() on it (as this snippet shows it)
My question: is there any design principle violation by implementing this pattern? (specially adopting the "shared" $sql property). If the answer is yes, why and what is the proper way to do it?
<?php
class ProductCodeDataSource
{
use dbmanager_aware_trait;
private $sql;
public function findByProductId($productId)
{
$this->sql = ;
$this->sql['fields'] = 'productcode_value';
$this->sql['tables'] = 'productcode';
$this->sql['where'] = sprintf('productcode_product_id = %d', (int) $productId);
$this->sql['order'] = 'productcode_default';
return $this;
}
public function findOneByProductId($productId)
{
$this->findByProductId($productId);
$this->sql['limit'] = 1;
return $this;
}
public function getResult()
{
// get_all_value is a legacy method that fetches data from mysql DB
// build_select_ext is also a legacy method that builds sql queries from
// $sql array structures
return$this->dbmanager->get_all_value($db->build_select_ext($this->sql));
}
}
php design-patterns solid-principles
asked Nov 26 '18 at 20:31
medunesmedunes
8311
add a comment |
0
I'm trying to write a class that relies on some legacy code to fetch data from database related to a particular table (productcode)
For not repeating myself, I implement a private property $sql that holds the "evolving" query and finally applying getResult() on it (as this snippet shows it)
My question: is there any design principle violation by implementing this pattern? (specially adopting the "shared" $sql property). If the answer is yes, why and what is the proper way to do it?
<?php
class ProductCodeDataSource
{
use dbmanager_aware_trait;
private $sql;
public function findByProductId($productId)
{
$this->sql = ;
$this->sql['fields'] = 'productcode_value';
$this->sql['tables'] = 'productcode';
$this->sql['where'] = sprintf('productcode_product_id = %d', (int) $productId);
$this->sql['order'] = 'productcode_default';
return $this;
}
public function findOneByProductId($productId)
{
$this->findByProductId($productId);
$this->sql['limit'] = 1;
return $this;
}
public function getResult()
{
// get_all_value is a legacy method that fetches data from mysql DB
// build_select_ext is also a legacy method that builds sql queries from
// $sql array structures
return$this->dbmanager->get_all_value($db->build_select_ext($this->sql));
}
}
php design-patterns solid-principles
asked Nov 26 '18 at 20:31
medunesmedunes
8311
How do you share the$sqlproperty? How does the methodbuild_select_extlook like? How much do you trust your inputs? Should you parameterize them?
– Dharman
Nov 26 '18 at 20:36
2
SO is not the most suitable place for asking code review question. There is a dedicated SE page: codereview.stackexchange.com
– Dharman
Nov 26 '18 at 20:37
as you see in the code "private $sql;" so it is just a private property and each findXXX method works on it to change it to a certain point, the build_select_ext is just turning this $sql array variable into a $sql string , inputs are perfectly trustworthy, and there is no need to parametize them
– medunes
Nov 26 '18 at 20:42
1
So you mean that you are reusing the object? How could this impact anything if you clear the$sqlarray each time you callfindByProductId? Building dynamic SQL statements is a common thing. Take a look at EasyDB class by Paragonie
– Dharman
Nov 26 '18 at 20:55
add a comment |
0
0
0
I'm trying to write a class that relies on some legacy code to fetch data from database related to a particular table (productcode)
For not repeating myself, I implement a private property $sql that holds the "evolving" query and finally applying getResult() on it (as this snippet shows it)
My question: is there any design principle violation by implementing this pattern? (specially adopting the "shared" $sql property). If the answer is yes, why and what is the proper way to do it?
<?php
class ProductCodeDataSource
{
use dbmanager_aware_trait;
private $sql;
public function findByProductId($productId)
{
$this->sql = ;
$this->sql['fields'] = 'productcode_value';
$this->sql['tables'] = 'productcode';
$this->sql['where'] = sprintf('productcode_product_id = %d', (int) $productId);
$this->sql['order'] = 'productcode_default';
return $this;
}
public function findOneByProductId($productId)
{
$this->findByProductId($productId);
$this->sql['limit'] = 1;
return $this;
}
public function getResult()
{
// get_all_value is a legacy method that fetches data from mysql DB
// build_select_ext is also a legacy method that builds sql queries from
// $sql array structures
return$this->dbmanager->get_all_value($db->build_select_ext($this->sql));
}
}
php design-patterns solid-principles
asked Nov 26 '18 at 20:31
medunesmedunes
8311
I'm trying to write a class that relies on some legacy code to fetch data from database related to a particular table (productcode)
For not repeating myself, I implement a private property $sql that holds the "evolving" query and finally applying getResult() on it (as this snippet shows it)
My question: is there any design principle violation by implementing this pattern? (specially adopting the "shared" $sql property). If the answer is yes, why and what is the proper way to do it?
<?php
class ProductCodeDataSource
{
use dbmanager_aware_trait;
private $sql;
public function findByProductId($productId)
{
$this->sql = ;
$this->sql['fields'] = 'productcode_value';
$this->sql['tables'] = 'productcode';
$this->sql['where'] = sprintf('productcode_product_id = %d', (int) $productId);
$this->sql['order'] = 'productcode_default';
return $this;
}
public function findOneByProductId($productId)
{
$this->findByProductId($productId);
$this->sql['limit'] = 1;
return $this;
}
public function getResult()
{
// get_all_value is a legacy method that fetches data from mysql DB
// build_select_ext is also a legacy method that builds sql queries from
// $sql array structures
return$this->dbmanager->get_all_value($db->build_select_ext($this->sql));
}
}
php design-patterns solid-principles
php design-patterns solid-principles
asked Nov 26 '18 at 20:31
medunesmedunes
8311
asked Nov 26 '18 at 20:31
medunesmedunes
8311
asked Nov 26 '18 at 20:31
medunesmedunes
8311
asked Nov 26 '18 at 20:31
medunesmedunes
8311
asked Nov 26 '18 at 20:31
medunesmedunes
8311
8311
How do you share the$sqlproperty? How does the methodbuild_select_extlook like? How much do you trust your inputs? Should you parameterize them?
– Dharman
Nov 26 '18 at 20:36
2
SO is not the most suitable place for asking code review question. There is a dedicated SE page: codereview.stackexchange.com
– Dharman
Nov 26 '18 at 20:37
as you see in the code "private $sql;" so it is just a private property and each findXXX method works on it to change it to a certain point, the build_select_ext is just turning this $sql array variable into a $sql string , inputs are perfectly trustworthy, and there is no need to parametize them
– medunes
Nov 26 '18 at 20:42
1
So you mean that you are reusing the object? How could this impact anything if you clear the$sqlarray each time you callfindByProductId? Building dynamic SQL statements is a common thing. Take a look at EasyDB class by Paragonie
– Dharman
Nov 26 '18 at 20:55
add a comment |
How do you share the$sqlproperty? How does the methodbuild_select_extlook like? How much do you trust your inputs? Should you parameterize them?
– Dharman
Nov 26 '18 at 20:36
2
SO is not the most suitable place for asking code review question. There is a dedicated SE page: codereview.stackexchange.com
– Dharman
Nov 26 '18 at 20:37
as you see in the code "private $sql;" so it is just a private property and each findXXX method works on it to change it to a certain point, the build_select_ext is just turning this $sql array variable into a $sql string , inputs are perfectly trustworthy, and there is no need to parametize them
– medunes
Nov 26 '18 at 20:42
1
So you mean that you are reusing the object? How could this impact anything if you clear the$sqlarray each time you callfindByProductId? Building dynamic SQL statements is a common thing. Take a look at EasyDB class by Paragonie
– Dharman
Nov 26 '18 at 20:55
How do you share the
$sql property? How does the method build_select_ext look like? How much do you trust your inputs? Should you parameterize them?– Dharman
Nov 26 '18 at 20:36
How do you share the
$sql property? How does the method build_select_ext look like? How much do you trust your inputs? Should you parameterize them?– Dharman
Nov 26 '18 at 20:36
2
2
SO is not the most suitable place for asking code review question. There is a dedicated SE page: codereview.stackexchange.com
– Dharman
Nov 26 '18 at 20:37
SO is not the most suitable place for asking code review question. There is a dedicated SE page: codereview.stackexchange.com
– Dharman
Nov 26 '18 at 20:37
as you see in the code "private $sql;" so it is just a private property and each findXXX method works on it to change it to a certain point, the build_select_ext is just turning this $sql array variable into a $sql string , inputs are perfectly trustworthy, and there is no need to parametize them
– medunes
Nov 26 '18 at 20:42
as you see in the code "private $sql;" so it is just a private property and each findXXX method works on it to change it to a certain point, the build_select_ext is just turning this $sql array variable into a $sql string , inputs are perfectly trustworthy, and there is no need to parametize them
– medunes
Nov 26 '18 at 20:42
1
1
So you mean that you are reusing the object? How could this impact anything if you clear the
$sql array each time you call findByProductId? Building dynamic SQL statements is a common thing. Take a look at EasyDB class by Paragonie– Dharman
Nov 26 '18 at 20:55
So you mean that you are reusing the object? How could this impact anything if you clear the
$sql array each time you call findByProductId? Building dynamic SQL statements is a common thing. Take a look at EasyDB class by Paragonie– Dharman
Nov 26 '18 at 20:55
add a comment |
0
active
oldest
votes
Your Answer
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53488607%2fis-there-a-problem-using-a-shared-private-property-in-a-class-to-build-fluent%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
0
active
oldest
votes
0
active
oldest
votes
active
oldest
votes
active
oldest
votes
draft saved
draft discarded
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
draft saved
draft discarded
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53488607%2fis-there-a-problem-using-a-shared-private-property-in-a-class-to-build-fluent%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
How do you share the
$sqlproperty? How does the methodbuild_select_extlook like? How much do you trust your inputs? Should you parameterize them?– Dharman
Nov 26 '18 at 20:36
2
SO is not the most suitable place for asking code review question. There is a dedicated SE page: codereview.stackexchange.com
– Dharman
Nov 26 '18 at 20:37
as you see in the code "private $sql;" so it is just a private property and each findXXX method works on it to change it to a certain point, the build_select_ext is just turning this $sql array variable into a $sql string , inputs are perfectly trustworthy, and there is no need to parametize them
– medunes
Nov 26 '18 at 20:42
1
So you mean that you are reusing the object? How could this impact anything if you clear the
$sqlarray each time you callfindByProductId? Building dynamic SQL statements is a common thing. Take a look at EasyDB class by Paragonie– Dharman
Nov 26 '18 at 20:55