• Post Reply Bookmark Topic Watch Topic
  • New Topic

Please help me find errors...  RSS feed

 
Yasser Shaikh
Greenhorn
Posts: 21
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
list all bad coding practices in the following code snippet

try{
for(int i=0;oldCode.size();i++){
String insertIntoMappingtbl="insert into schemaptbl '' + " values(?,?,?,?,?,?)";
pstmtMappingTbl=con.prepareStatement(insertIntoMap pingTbl );
pstmtMappingTbl.setObject(1,oldCode.get(i));
pstmtMappingTbl.setObject(2,oldId.get(i));
pstmtMappingTbl.setObject(3,newCode.get(i));
pstmtMappingTbl.setObject(4,newId.get(i));
pstmtMappingTbl.setObject(5,sub.get(i));
pstmtMappingTbl.setObject(6,subid.get(i));
pstmtMappingTbl.executeQuery();
}
}
catch(Exception e){}
finally{return success;}
 
Wouter Oet
Bartender
Posts: 2700
IntelliJ IDE Opera
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What do you think? We are not an answering machine. And please UseCodeTags. You can edit your post with the button.
 
Mohamed Sanaulla
Bartender
Posts: 3185
34
Google App Engine Java Ruby
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Please use the CODE tags to post your source code.

-- Also as you are doing an INSERT operation- better use-executeUpdate instead of executeQuery.
-- I see that you need not concatenate the String used in the SQL. Also you can move the Query string out of the loop.
-- Your try...catch isnt doing any thing- You need to catch the exceptions which are thrown and also print/log some message/stacktrace in the catch. (Instead of using a catch block with Exception, you can use a catch block with SQLException or which ever Exception is being thrown)
-- You need to always close the Database resource in the finally clause.
--With in the for loop- I dont know why you are using- setObject. You can use set<DataType> method depending on the data type of the value being set.

Adding to this- As the code isn't complete- We don't know from where the other variables(oldId, newCode ...) are coming from. Summarizing- The lines with in the code are really confusing.

Always use meaningful variable names.

Update: I happened to post this, just before I could see Wouter's reply.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!