Want to convert my code in method

[Farakh khan]

I am using java servlet with Oracle XE

my pagging code is working fine but due to my inexperty I am not able to convert my pagging code into function to reuse it. Please help me

My code

import java.io.*;
import java.sql.*;
import javax.servlet.*;
import javax.servlet.http.*;

public class CVs extends HttpServlet{

	Connection 	cnn=null;
	ResultSet 	 rs=null;

public void doGet(HttpServletRequest req, HttpServletResponse res)throws IOException, ServletException{

	res.setContentType("text/html");
	PrintWriter out=res.getWriter();

try{
	if ((cnn==null)||cnn.isClosed()){
	cnn=DB.getOracleConnection();
	}

	out.print("<link rel=stylesheet href=/css/khan.css type=text/css media=screen>");

	WebPage wp=new WebPage();//website graphic interface

//Pagging part-1 started

	int start=0,end=0,currentPage=0,next=0,previous=0,pages=0,totalPages=0,lastPage=0,i=0;

	String pageNo=req.getParameter("pageNo");
	if(pageNo != null){
	currentPage=Integer.parseInt(pageNo);

	String query="select count(*) from isCvPublished where status=2";


	int totalRecords=0;

	PreparedStatement ps=cnn.prepareStatement(query);
	
	rs=ps.executeQuery();
	if(rs.next()){
	totalRecords=rs.getInt(1);
	}
	ps.close();

	totalPages= (totalRecords / 14) +1;
	end=(currentPage * 14);
	start=(end-14) +1;	
	next=currentPage+1;
	previous=currentPage-1;
	lastPage=totalPages;

		out.println(wp.pagePrint1()+
		"<table border=0><tr><td class=gull width=230>CVs</td><td width=410 align=center>");
		
		if((previous<=totalPages) && (previous>0)){
		out.print("<a href=/servlet/CVs?pageNo="+previous+">Previous</a>"+
		"<font size=2 color=blue>«|</font>  ");
		out.println(" <a href=/servlet/CVs?pageNo=1><b>1..,  </b></a><b></b>");
		}
			
	if(totalPages>5 && totalPages<=lastPage){	
		if(currentPage==1){
		out.println("<a href=/servlet/CVs?pageNo=1><b>1</b></a>  "+
		"<a href=/servlet/CVs?pageNo=2><b>2</b></a>  "+
		"<a href=/servlet/CVs?pageNo=3><b>3</b></a>  "+
		"<a href=/servlet/CVs?pageNo=4><b>4</b></a>  "+
		"<a href=/servlet/CVs?pageNo=5><b>5</b></a>");
		}
	

		else{
			for(i=1;i<=5;i++){
			out.print("  "+
			"<a href=/servlet/CVs?pageNo=" + ((currentPage+i)-1) +"><b>"+
			((currentPage+i)-1)+"</b></a>");

			if((currentPage+i)>=lastPage){
			break;		
		}	}
		}
		}else{
			if((totalPages * 5) +1>=totalRecords){
			totalPages++;
			}

			for(i = 1; i < totalPages; i++){
			out.print("   <a href=/servlet/CVs?pageNo=" + i +">"+ i +"</a>");
			}
		}
		
		if((next<totalPages) && (totalPages>1) && (currentPage<lastPage)){
		out.print("  <font size=2 color=blue>|» </font>"+
		"<a href=/servlet/CVs?pageNo="+(currentPage+1)+">Next</a>     <br>");
		}
		out.println("</td></tr></table>"+

//pagging part-I ended
		
			"<table border=1 frame=border rules=none bordercolor=#4A659C "+
			"cellspacing=3 width=99% cellpadding=5>"+

			"<tr bgcolor=#4A659C>"+
			"<td align=center><font color=white><b>Idustry</b></td>"+
			"<td width=41 align=center><font color=white><b>Age</b></td>"+
			"<td width=50 align=center><font color=white><b>Experience</b></td>"+
			"<td width=86 align=center><font color=white><b>Country</b></td></tr>");
	
	MajorSubject ms=new MajorSubject();

	int count=0; String color="";


////////////////////////Pagging "start" and "end" variable used here with the oracle statement
	
	String query1="select * from(select zafar.*,rownum khan from( "+
		" select "+
			" a.emp_id,to_char(dob,'dd'),to_char(dob,'mm'),to_char(dob,'yyyy'),"+
			"country_of_Living,category,total_exp  "+
		" from "+
			" isCvPublished a, employee b, postCV c "+
		" where  "+
			" a.emp_id=b.emp_id and a.emp_id=c.emp_id and a.status=2 "+
		" order by "+
			" p_date desc"+
		" ) zafar where rownum<="+end+") where khan>="+start ;
//////////////////////

	PreparedStatement ps1=cnn.prepareStatement(query1);
	rs=ps1.executeQuery();
	
	while(rs.next()){
	String cv_id=rs.getString(1);
	String day=rs.getString(2);
	String month=rs.getString(3);
	String year=rs.getString(4);
	String country=rs.getString(5);
	String industry=rs.getString(6);
	String exp=rs.getString(7);

	count+=1;
	
		
		if(count%2==0){
		color="bgcolor=#EEEEEE   onMouseOver=\"this.bgColor='#C0D9D9';\""+ 
		"onMouseOut=\"this.bgColor='#EEEEEE';\"	";					
		}else{
		color="\"style=background:transparent\"; onMouseOver=\"style.backgroundColor=\'#C0D9D9\';\" "+
		"onmouseout=\"style.backgroundColor=\'transparent\';\"";
		}


	out.println("<tr "+color+">"+
	"<td><a href=/servlet/DisplayCVs?cv_id="+cv_id+"&link=1&job_id=0>"+
	Industry.displayIndustry(industry)+"</td>"+

	"<td width=41 align=center><a href=/servlet/DisplayCVs?cv_id="+cv_id+"&link=1&job_id=0>"+
	CalculateAge.AgeCalculator(day,month,year)+
	"</a></td>"+

	"<td width=50 align=center><a href=/servlet/DisplayCVs?cv_id="+cv_id+"&link=1&job_id=0>"+exp+" Years</a></td>"+

	"<td width=86><a href=/servlet/DisplayCVs?cv_id="+cv_id+"&link=1&job_id=0>"+country+"</a></td></tr>");
	
	}	
	ps1.close();

	out.println("</table>"+wp.pagePrint2());

//Paging part-II started

	}
	else{
	res.sendRedirect("/servlet/CVs?pageNo=1");
	}
	
//Paging part-II ended

	
	
}catch(Exception e){
	out.println("CVs.java "+e.toString()); 
	System.out.println("CVs.java "+e.toString());  
}

finally  {      
	try {        
		if ( rs  != null ) 	rs.close();            
		if ( cnn != null )  	cnn.close();      
	} catch(SQLException ex) {          
	out.println("CVs..java "+ex.toString()); 
	System.out.println("CVs..java "+ex.toString());  
	}
}
		
	}
}

Best Regards

[David Newton]

Start by refactoring--it will make things easier to pull out once you have some separate pieces to start with.

[Farakh khan]

David Newton wrote:Start by refactoring--it will make things easier to pull out once you have some separate pieces to start with.

I tried to divide and commented like Pagging part-1 starts here and end here.similarly pagging part-2 started & ended

Is that you please mean?? Sorry as my English is not good

Best Regards

[David Newton]

Not what I meant, but on the right track--but instead of just putting comments between sections, take the "sections" out and put them in to new methods.

The Wikipedia "refactoring" article gives a broad overview. In this case, with your code, I would start by pulling out small "chunks" of functionality into separate methods. This has two benefits:

- The code that *doesn't* deal with paging becomes easier to read

As you do that, you'll also find a good way (or several good ways, and occasionally some bad ways :) to make the paging code work for any type of object, on any page, and so on.

By the way, having the connection and result set be instance variables is a really bad idea--servlets must be thread safe.

[David Newton]

For what it's worth, after some refactoring, I'm up to 11 methods, and am at the point where the abstraction/extension points are pretty clear, assuming we're generating our HTML in a servlet (not a great idea).

The paging logic (somewhat convoluted, has at least one strange redundancy) is around 150 lines (too long!). If getting the total record count, and paging link URL generation, the code is marginally general-purpose as-is.

[Farakh khan]

David Newton wrote:For what it's worth, after some refactoring, I'm up to 11 methods, and am at the point where the abstraction/extension points are pretty clear, assuming we're generating our HTML in a servlet (not a great idea).

Thanks for your favorable reply. 11 methods!!! great I am not thinking even a single method due to my incompetency. Can you please explain why generating HTML with servlet is not a good idea?

2) can you please copy paste some of your developed methods from 11 methods

David Newton wrote:
By the way, having the connection and result set be instance variables is a really bad idea--servlets must be thread safe.

Can you please explain it little bit more as to why I not instantiate them?

Best Regards

[Saifuddin Merchant]

Farakh khan wrote:
Can you please explain it little bit more as to why I not instantiate them?

Here is a good explanation -- http://www.javaworld.com/javaworld/jw-07-2004/jw-0712-threadsafe.html

[Farakh khan]

Sam Mercs wrote:

Farakh khan wrote:
Can you please explain it little bit more as to why I not instantiate them?

Here is a good explanation -- http://www.javaworld.com/javaworld/jw-07-2004/jw-0712-threadsafe.html

Thanks Sam for your favorable reply but my original question is still unanswered

Best Regards

[Saifuddin Merchant]

True -- But before you actually make it re-usable the first thing do to would be Separate the View from the Model. Look up for MVC and JSP to get some idea. Once you have some of the view separated out from the code -- you'll see that it is pretty manageable...

Also look for paging in JSP -- you should find some code snippets that would help you ...

[Farakh khan]

Sam Mercs wrote:True -- But before you actually make it re-usable the first thing do to would be Separate the View from the Model. Look up for MVC and JSP to get some idea. Once you have some of the view separated out from the code -- you'll see that it is pretty manageable...

Also look for paging in JSP -- you should find some code snippets that would help you ...

I am learning. Really I learned a lot at this forum

Please keep this thread open and I'll back to you

Best Regards

[Farakh khan]

Sam Mercs wrote:True -- But before you actually make it re-usable the first thing do to would be Separate the View from the Model. Look up for MVC and JSP to get some idea. Once you have some of the view separated out from the code -- you'll see that it is pretty manageable...

Also look for paging in JSP -- you should find some code snippets that would help you ...

I tried a lot but unable to fix the problem

Can you please split my code according to MVC. that'll be very helpful to start with MVC

Best Regards

[David Newton]

How about showing us what you've done so far? Break out the functionality piece-by-piece: we're not going to do it for you, but we're happy to provide hints, etc. For example, here's a list of the methods I created during my first draft refactoring:

buildPagingTable
buildResultRows
closeDbArtifacts
countTotalRecords
createQuery
cvsUrl
cvsUrl
displayUrl
doGet
getCurrentPageNo
writeRow