4
\$\begingroup\$

I don't know it is some design pattern for it or I can't do it in a different way the code looks like this:

Iteam iteam = itemRepository.findByNameAndNoAndCountryCodeAndCurrencyCode(name, countryCode, currencyCode);
    if(item == null){
        item = itemRepository.findByNameAndNoAndCountryCodeAndCurrencyCode(name, "US", currrencyCode)
    }
    if(item == null){
        item = itemRepository.findByNameAndNoAndCountryCodeAndCurrencyCode(name, countryCode, "USD")
    }
    if(item == null){
        item = itemRepository.findByNameAndCountryCode(name, countryCode)
    }
    if(item == null){
        item = itemRepository.findByNameAndCurrencyCode(name, currrencyCode)
    }

And I have a lot of this (like 15) kind of queries with the condition it is possible to do not make to many conditions and use some trick for this code?

\$\endgroup\$
  • 3
    \$\begingroup\$ Code Review requires concrete code from a project, with enough code and / or context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. Your current code is an example, generalized code. While other sites prefer that, Code Review actually requires the details of your actual code. \$\endgroup\$ – Mast Sep 16 at 7:26
  • \$\begingroup\$ @Mast I think it is enough because If I copy and paste all of my code I will add only 11 more conditions but with different queries \$\endgroup\$ – Mateusz Sobczak Sep 16 at 7:29
  • \$\begingroup\$ @mateusz-sobczak Mast is right, they’re not enough stuff, at the moment, to do a proper review, I suggest that you provide more information about the structure of the database (tables, provider of the database (Oracle, MSQL, etc.)) and try to explain more the business logic of the queries. \$\endgroup\$ – Doi9t Sep 16 at 12:45
4
\$\begingroup\$

As suggested by @911992, why not moving that "logic" into the database. Not as a function/stored-procedure/view but as a combination of all your queries :

SELECT * FROM items WHERE name=? AND country_code=? AND currencyCode=?
UNION
SELECT * FROM items WHERE name=? AND country_code='US' AND currencyCode=?
UNION
SELECT * FROM items WHERE name=? AND country_code=? AND currencyCode='USD'
UNION
SELECT * FROM items WHERE name=? AND country_code=?    
UNION
SELECT * FROM items WHERE name=? AND currencyCode=?

You can limit that query to one row to avoid too much results.

If want to keep everything into the code and are looking for a design pattern, you can search for the chain of responsibility.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ In my opinion, this is the best way, but, I think it would be best to add another column that indicate the origin of the result (first query, second, etc.). SELECT 'BY_SELECTED_COUNTRY' as ORIGIN,* FROM items WHERE name=? AND country_code=? AND currencyCode=? So you would be able to filter the result (if you have rows in more than a query and want to remove the others); I’m so rusted in sql, but I think this would work. \$\endgroup\$ – Doi9t Sep 16 at 12:36
  • \$\begingroup\$ Indeed, but if the UNION returns the results in the same order as the declaration of the query, then the first line will always be the "best" one. Anyway, safety is always a good thing. \$\endgroup\$ – gervais.b Sep 16 at 14:07
2
\$\begingroup\$

Keeping all data-related functionalities and stuff in correct/related layer will help you to keep the business(data-consumer) layer more simple, and easier to maintain.

But in otherside, implementing some complex functionalities over data layer(assuming SQL) might be very challenging. So a good balance should be there.

About your case, since it looks like a chain of queries to find out some result(no complex stuffs), I suggest move all of these calls into one SQL function/procedure that will bring some notable gifts for you, such as less mess on business layer, and probably a huge performance improvements.

Sample pgsql query

--drop function item_search(...)
create or replace function item_search(
text,/*name*/
text,/*country code*/
text,/*currency code*/
)returns table(
_name text,
_cnt_code text,
_cur_code text
) as $$
/*first attempt, hoping it has results*/
with _sel0 as (select * from items where "name" = $1 and country_code=$2 and currency_code=$3 limit 11),
/*counting the result of ^^*/
_sel0_c as (select count(*) as "c" from _sel0),
/*perform next query, if _sel0 ^ was empty(_sel0_c.c should be 0), using US for country code*/
_sel1 as (select i.* from items i,_sel0_c _s0c where _s0c."c" = 0 "name" = $1 and country_code="US" and currency_code=$3 limit 11),
.../*counting the above, and perform next query(s)*/
...
...
/*finally union all results*/
select * from _sel0
union all
select * from _sel1
union all
...
union all
select * from _seln
$$ language sql;

Above function, performs the same code you provided, but in sql/data layer. It certainly works faster than same impl in business-layer.

Now in your business layer, you have to call only one function, with required params, and only one result nullify check which keeps your business layer clean.

Business Layer(ORM/DOA) Solution

But if you are still insist to make in with business-layer(ORM/DOA), it's still possible(but I think it doesn't worth at the end). AM not sure if common APIs(like JPA, whatever) support for such thing since again, it's not common, but you would have you api-impl over working ORM API, to make it possible.

You could go for something like Value type that has a pair of values, one as expected value, and one as default(when expected results null)

Something like following

public interface Value<A>{
  A get_expected_value();
  A get_default_value();
}
//impls
public class StringValue<String>{
...
}

Now implement your dedicated DOA manager, where performs a query based on expected values first, and it go for default values when expected resulted empty/null results.

At the end you come up with something like your code, more complicated, but a with a nice/easy API. But again, leave it to the SQL.

Overall

Object oriented is nice, ok. Design patterns make it much interesting, for sure. DOA/ORM is all essential for database stuffs, NO!

ORM is nice, but as you take more high-level API for doing things, you take it easier for most common cases, but usually it' a mess(not really worth it) when it's not common(like your case).

| improve this answer | |
New contributor
911992 is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
  • 1
    \$\begingroup\$ I don't know why this answer got a downvote. I would not suggest a function but one huge query that combine all the possible cases would be a good solution. \$\endgroup\$ – gervais.b Sep 16 at 10:59
0
\$\begingroup\$

You should ideally rewrite the ItemRepository class to return Optional and never null. Then, you can create a chain of or.

| improve this answer | |
New contributor
asdofindia is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
\$\endgroup\$
  • 3
    \$\begingroup\$ It might be better not to answer questions that have a comment indicating it is off-topic. \$\endgroup\$ – pacmaninbw Sep 16 at 11:55
  • \$\begingroup\$ Hello, in my opinion, it's a better choice to throw a check exception (NoSuchElementException, ect) in the repository layer in case the value is present or not. This method is more appropriate, since it will remove the need to always unwrap the Optional and recheck if the value is present or not in the service layer. \$\endgroup\$ – Doi9t Sep 16 at 11:57

Not the answer you're looking for? Browse other questions tagged or ask your own question.