Path: csiph.com!fu-berlin.de!uni-berlin.de!individual.net!not-for-mail
From: Robert Klemme <shortcutter@googlemail.com>
Newsgroups: comp.lang.java.programmer
Subject: Re: Where to put the return
Date: Sat, 19 Aug 2017 11:10:22 +0200
Lines: 90
Message-ID: <evqdjvFbc1oU1@mid.individual.net>
References: <87zib6zeup.fsf@munus.decebal.nl> <omlidd$k9h$1@gioia.aioe.org> <ev9dm4Ff34aU1@mid.individual.net> <87y3qlnyvj.fsf@munus.decebal.nl>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
X-Trace: individual.net jWk0xGgNN18MrSpu6fgh6Q/VnHWB7tB21eCu4BzvksJPVVvRk=
Cancel-Lock: sha1:xMf6TkIgkef/vxagdUlwTLGMxeI=
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1
In-Reply-To: <87y3qlnyvj.fsf@munus.decebal.nl>
Content-Language: en-GB
Xref: csiph.com comp.lang.java.programmer:37280

On 15.08.2017 08:12, Cecil Westerhof wrote:
> Robert Klemme <shortcutter@googlemail.com> writes:
> 
>> Btw. I'd rather throw in catch than return a special value.  That is
>> cleaner and does not break if the special value actually occurs in the
>> database.
> 
> I agree. But that is what the call to fatalError does: it cleans up
> resources and exits the program. But I changed that to just let the
> SQLException fire:
>      private static String getOldestUsed() throws SQLException {
>          String oldestUsed = "";
> 
>          try (SQLResultSet rsOldest = sqlite3Conn.executeQuery(OLDEST_USED)) {
>              if (rsOldest.next()) {
>                  oldestUsed = rsOldest.getString("used");
>              }
>          }
>          return oldestUsed;
>      }

I'd rather

private static String getOldestUsed() throws SQLException {
     try (final SQLResultSet rs = sqlite3Conn.executeQuery(OLDEST_USED)) {
         if (rs.next()) {
             return rs.getString("used");
         }
     }

     return "";
}


But religious wars have been fought about additional returns inside 
methods.  To me this reads easier because you see the fallback return 
value immediately and you can recognize immediately what value is 
returned.  In this smallish example it probably does not make a 
difference though.

Btw. SQL statements when used like this tend to be specific to the 
method.  So by inserting the SQL directly in the method instead of in a 
constant the reader has more information at hand and can quicker 
understand the logic.  Constants make much more more sense if the SQL is 
actually reused in different locations.

>> And another remark: we do not see the SQL but in this case I would make
>> sure that OLDEST_USED actually returns exactly one column and access
>> that via index instead of name.  IIRC this tends to be slightly quicker.
>> But it may be just an old habit of mine.
> 
> One of the things I do is make sure you cannot use index. I find that
> a very bad habit.

Why?

> Also when you give the wrong query, but it gives one
> column back, you have a hard to find bug.

If the value happens to be of the same type as the expected one, yes. 
But since I would insert the query text in the method (see above) that 
is less of an issue.

> The SQL used:
>      SELECT used
>      FROM   usedProverbs
>      LIMIT  1
> 
> I defined usedProverbs as a sorted view (on proverbs), that is why
> there is no SORT BY.

Sorted view is another thing I would not do.  I am not sure that is even 
a supported features across most databases.  It does have issues - 
technical and conceptual.

With the SQL given the reader does not have a chance to identify the 
full criteria that select the value "used" because you do not see the 
sorting criteria.  I would rather put the "ORDER BY" into the query. 
(Or make it a query with "MIN" depending on schema.)  You can read a lot 
on the issues of sorting a view, for example here:

https://dba.stackexchange.com/questions/21434/why-does-order-by-not-belong-in-a-view/

Kind regards

	robert

-- 
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/