This week's book giveaway is in the Artificial Intelligence and Machine Learning forum.
We're giving away four copies of Zero to AI - A non-technical, hype-free guide to prospering in the AI era and have Nicolò Valigi and Gianluca Mauro on-line!
See this thread for details.
Win a copy of Zero to AI - A non-technical, hype-free guide to prospering in the AI era this week in the Artificial Intelligence and Machine Learning forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Paul Clapham
  • Bear Bibeault
  • Jeanne Boyarsky
Sheriffs:
  • Ron McLeod
  • Tim Cooke
  • Devaka Cooray
Saloon Keepers:
  • Tim Moores
  • Tim Holloway
  • Jj Roberts
  • Stephan van Hulst
  • Carey Brown
Bartenders:
  • salvin francis
  • Scott Selikoff
  • fred rosenberger

One method to extract data from MySql Database

 
Ranch Hand
Posts: 176
4
Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello Everyone, I have made some progress I wanted to share. So I have this simple table in MySQL called manufacturers with to two columns only. An auto incremented ID integer and a the manufacturer's name.

I created this method that would retrieve the data from the database which I lightly tested and seems to work perfectly as required. I would like your professional input on how good this method is and how to make it better.

PS. I wanted to avoid having multiple methods to extract data. This is why I created just the one as you see. Thank you in advance for all the support.

 
Master Rancher
Posts: 4699
49
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You don't close any of your resources off in a finally block.
Indeed the only one you do close at all is ResultSet.

Since this appears to be a desktop app, I assume there's just the one Connection, so closing that is not so important.

Line 56 (and associated 53).
Does this work?
I'm fairly sure you can't treat a column name in a WHERE clause as a bind variable.

Personally I'm not keen on treating 0 as an null id. It's a perfectly valid value for an id.
Null would be much better.

That's the immediate code parts.

Design-wise...since I can only get a meaningful search by providing either an id or a name, then why expose this method at all?
I would have two methods that call this one, one for searchById and one for searchByName, which could call this one which does the actual work.

Indeed I would possibly even remove the if statement entirely and shift that up into the relevant method and they can pass the WHERE clause down into here.
That would remove a bunch of parameters and if clauses.
 
Saloon Keeper
Posts: 6625
161
Android Mac OS X Firefox Browser VI Editor Tomcat Server Safari
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Yosuf Ibrahim wrote:I wanted to avoid having multiple methods to extract data.


Why? What's wrong with having multiple DB access methods? It's good practice to have meaningful names for methods (and everything else, of course) - sql_retrieveFromDB says very little, and in any sizable app there must be lots of methods that retrieve data from a DB.
 
Saloon Keeper
Posts: 22634
153
Android Eclipse IDE Tomcat Server Redhat Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Close the Connection. I don't care if it's a desktop app or not. Desktop apps can leak resources and explode too. If you close the Connection, that automatically closes its Statement(s), which in turn closes the ResultSet(s). And close the Connection in a "finally" clause, just to ensure that failures don't cause the close operation to get accidentally bypassed (a very common mistake).

I'm afraid that I view String.format's in PreparedStatements with misgivings. Generally, a PreparedStatement is going to eliminate the need for special formatting. String formatting SQL in an invitation to SQL Injection attacks.

If the database really does have multiple column names for Manufacturer ID and Manufacturer name, the database schema design really needs to be reviewed. That's generally neither necessary nor desirable. And it certainly complicates the database logic.

 
Marshal
Posts: 70604
287
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Tim Holloway wrote:. . . close the Connection in a "finally" clause . . .

Since Connection implements AutoCloseable wouldn't you use try with resources instead?
 
Tim Holloway
Saloon Keeper
Posts: 22634
153
Android Eclipse IDE Tomcat Server Redhat Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:

Tim Holloway wrote:. . . close the Connection in a "finally" clause . . .

Since Connection implements AutoCloseable wouldn't you use try with resources instead?



Yes, I would, but I can never remember what it's called. Either solution works, though.
 
Campbell Ritchie
Marshal
Posts: 70604
287
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Tim Holloway wrote:. . . I can never remember what it's called. . . . .

Hahahahahahahahahahahahahaha!
 
Yosuf Ibrahim
Ranch Hand
Posts: 176
4
Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Dave Tolls wrote:
Line 56 (and associated 53).
Does this work?
I'm fairly sure you can't treat a column name in a WHERE clause as a bind variable.



I never tested it but doing that now it did not. Fixed it

Thank you for letting me know

Dave Tolls wrote:
Personally I'm not keen on treating 0 as an null id. It's a perfectly valid value for an id.
Null would be much better.



Fixed that as well and chose -1 as a null value instead even though I do not have any id's that are 0 and never will it caused me a different issue when searching for an Id that contains a 0

Dave Tolls wrote:
Why? What's wrong with having multiple DB access methods? It's good practice to have meaningful names for methods (and everything else, of course) - sql_retrieveFromDB says very little, and in any sizable app there must be lots of methods that retrieve data from a DB



Tim Moores wrote:
Why? What's wrong with having multiple DB access methods? It's good practice to have meaningful names for methods (and everything else, of course) - sql_retrieveFromDB says very little, and in any sizable app there must be lots of methods that retrieve data from a DB.



For every class that has data saved in the database I want only one method for each that extracts data. This makes it easier for me to edit the methods in case I changed something in the database structure during development, or I found an error. Isn't it good programming to minimize my code as much as possible and not have repeated code as much as possible?

Plus, I will expand this class later with more variables, so having multiple classes to edit will be extremely tiresome and redundant in my opinion, however I am still a newbie, so it is possible I would be wrong. I have other objects such as "Product" that have many more variables and I wanted to use the same concept. i know it is difficult to perfect, but once perfected I am sure it would greatly minimize space for error.

Tim Holloway wrote:
Close the Connection. I don't care if it's a desktop app or not. Desktop apps can leak resources and explode too. If you close the Connection, that automatically closes its Statement(s), which in turn closes the ResultSet(s). And close the Connection in a "finally" clause, just to ensure that failures don't cause the close operation to get accidentally bypassed (a very common mistake).



Closing the connection would be a disastrous mistake since then I would have to open it every time I want to run a query. The way I have it is I open the Connection on login and on closing the application I close the connection. If there is a better way please let me know (Which probably there is ;D)

Tim Holloway wrote:
If the database really does have multiple column names for Manufacturer ID and Manufacturer name, the database schema design really needs to be reviewed. That's generally neither necessary nor desirable. And it certainly complicates the database logic.



The DB definitely does not have multiple names for the columns. I can do dump mistakes but not idiotic mistakes. Manufacturer has two columns: "manufId" and "manufName" only. I may probably increase columns if I believe I would need more data.

-------------------------------------------------------------
Here is my new method after editing:



 
Tim Holloway
Saloon Keeper
Posts: 22634
153
Android Eclipse IDE Tomcat Server Redhat Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The value NULL in a DBMS means "no data available". It's a much better thing to use than a "magic" value like 0 or -1.

I reiterate: Close the connection. Connections demand overhead from the application's OS/network stack, the JVM and the database server.  If you're running an app where you make frequent repeated requests, yes, it's better to obtain the Connection before you start and use it until you're done, but if multiple threads need Connections (like webapps do), then use a Connection Pool,  If long intervals occur between data requests, close the Connection and open a new one when you need it. I'd be more inclined to include the Connection as a parameter to the data access method than to look like I'm using a DataSource when I'm not. DataSource connections are always closed after use (the close method sends the Connection back to the pool). The main thing is it's not a good habit to obtain a resource in one place and dispose of it in another.

Definitely do not rely on garbage collection to close Connections or files. I used to work with someone who did until one day we had to clean it all up. If you are keeping a Connection long-term, close the Statement. As I said, that will close its associated ResultSet(s).

And speaking of which, if all you want is a "findByID/findByName/findByWhatever", function to return results, then instead of making up dynamic SQL statements in a web of conditions, it's much cleaner and simpler to implement a DAO class with "finder" methods in it. DAOs are a proven JEE design pattern. I generally have a separate DAO for each table I access and it contains the CRUD and finder primmitives. A service layer above the DAOs manages cases where multiple related tables are dealt with.
 
Marshal
Posts: 25930
69
Eclipse IDE Firefox Browser MySQL Database
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I really don't understand why you are using ResultSet.CONCUR_UPDATABLE, since you close the ResultSet immediately after reading it and building a list from it. What's the point?
 
Yosuf Ibrahim
Ranch Hand
Posts: 176
4
Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Paul Clapham wrote:I really don't understand why you are using ResultSet.CONCUR_UPDATABLE, since you close the ResultSet immediately after reading it and building a list from it. What's the point?



I honestly have no idea :l
 
Yosuf Ibrahim
Ranch Hand
Posts: 176
4
Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Tim Holloway wrote:The value NULL in a DBMS means "no data available". It's a much better thing to use than a "magic" value like 0 or -1.



That 0 and -1 is used only with the application not the DBMS. I cannot store a null in a java-numeral value so I have to turn it into a numeral. And BTW all that other stuff you mentioned (Connection Pools, DAO, CRUD & finder primitives), I have no idea what they are, not a clue, so I am going to study them a bit then get back to you with questions :P

Man I was so proud of the progress I made, in one single reply you showed me how little I know, haha.

Thanks guys for all the time you put in helping us newbies out here. I really have no idea how far I would have been without your support.
 
Paul Clapham
Marshal
Posts: 25930
69
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Yosuf Ibrahim wrote:That 0 and -1 is used only with the application not the DBMS.



You only need the 0 or -1 as a signal which says "Actually I don't have a manufacturer ID for you to search for". My suggestion would be to have two methods: one to search by manufacturer ID and the other to search by manufacturer name, since only those two things are actually supported by your single method.

That would make the code tremendously simpler. The search-by-ID method would have a single SQL query, no need to build it by string concatenation. And the search-by-name method would have two SQL queries, which you would choose between them based on the boolean parameter. Both methods would create a PreparedStatement from one of the three queries and pass it to another method which would execute it and build a list of results.
 
Yosuf Ibrahim
Ranch Hand
Posts: 176
4
Eclipse IDE Java Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Paul Clapham wrote:You only need the 0 or -1 as a signal which says "Actually I don't have a manufacturer ID for you to search for".



I am going to use -1
 
Paul Clapham
Marshal
Posts: 25930
69
Eclipse IDE Firefox Browser MySQL Database
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Yosuf Ibrahim wrote:

Paul Clapham wrote:You only need the 0 or -1 as a signal which says "Actually I don't have a manufacturer ID for you to search for".



I am going to use -1



Makes no difference. If somebody uses that method and passes it both a valid manufacturer ID and a valid manufacturer name, I don't know what they would expect to happen but in practice the name will be ignored because your method doesn't handle that situation. That's why I suggested you need to break it into two methods.
 
Tim Holloway
Saloon Keeper
Posts: 22634
153
Android Eclipse IDE Tomcat Server Redhat Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Yosuf Ibrahim wrote:

Tim Holloway wrote:The value NULL in a DBMS means "no data available". It's a much better thing to use than a "magic" value like 0 or -1.



That 0 and -1 is used only with the application not the DBMS. I cannot store a null in a java-numeral value so I have to turn it into a numeral.



Actually, you can and it is very, very common when working with databases. While an integer or floating-point primitive cannot be null, a java.lang.Integer can in fact be null. Although when I checked the JDBC ResultSet API, I see that getInt() returns a primitive integer, which is rather strange. But the JavaDocs do say that getInt returns 0 if the column value is null. If you absolutely have to know whether it's null or zero, you could use getObject() instead.

But otherwise, the best choice for "magic value" looks to be zero. Take note, however, that if you read a null field and update it based on what you read (0), then the database table would no longer have a null value and that could be a problem when working with other languages than Java or with database utilities.
 
What I don't understand is how they changed the earth's orbit to fit the metric calendar. Tiny ad:
Thread Boost feature
https://coderanch.com/t/674455/Thread-Boost-feature
reply
    Bookmark Topic Watch Topic
  • New Topic