Bug 44972 - [extract method] handle single exit
Summary: [extract method] handle single exit
Status: ASSIGNED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P4 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-10-15 19:06 EDT by Brett Kotch CLA
Modified: 2006-06-12 05:51 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Brett Kotch CLA 2003-10-15 19:06:20 EDT
Extracting the following code to method "resoucesNotSelected" :

protected boolean isEnabled() 
{
[begin extract here]
	boolean noResourcesSelected = selectedFiles.size() == 0;
	if (noResourcesSelected) 
	{
		return false;		
	}

[end extract here]
    ...

should genearate the following code: 
     
       
       if (resoucesNotSelected()) 
       {
            return false;
       }
Comment 1 Martin Aeschlimann CLA 2003-10-16 04:56:37 EDT
Looking at the original selection I'm puzzeled how you would end up with the
wanted result.
What would be the rule? Why would you ignore half or the selection?
Comment 2 Brett Kotch CLA 2003-10-16 12:53:53 EDT
Perhaps a different example would help. 

I have the following code in method isEnabled() ... 

		IPerforceServer server = ((P4File)selectedFiles.get
(0)).getServer();
		IChangelist changelist = PendingChangelistCache.getChangelist
(server, ((P4File)selectedFiles.get(0)).getChangelistId());
		
		String serverClient = server.getClient();
		String changelistClient = changelist.getClientName(); 
		
		if (!serverClient.equals(changelistClient))
		{
			return false;
		}

I would like to select the whole block and extract method doesntOwnChangelist. 
selectedFiles is an attribute of the class isEnabled is defined in.  

The result of extract method in isEnabled would then be:
   if (doesntOwnChangelist()) {
         return false;
   }

Since there is only a single exit in the code to be extracted, and it is false, 
it can eaisly be replaced in isEnabled. 

The method submitDoesntOwnChangelist would appear as such (excuse the 
indentation):

private boolean submitDoesntOwnChangelist() {

		IPerforceServer server = ((P4File)selectedFiles.get
(0)).getServer();
		IChangelist changelist = PendingChangelistCache.getChangelist
(server, ((P4File)selectedFiles.get(0)).getChangelistId());
		
		String serverClient = server.getClient();
		String changelistClient = changelist.getClientName(); 
		
		if (!serverClient.equals(changelistClient))
		{
			return false;
		}

		return true; // ADDED BY EXTRACT SINCE THERE IS A SINGLE EXIT.
                             // The resulting exit must be the opposite of the 
                             // existing exit. 

 }


Such a pattern is very common when coding guard clauses ... but yet those guard 
clauses should be extracted into sep methods.