Bug 44972

Comment 1 Martin Aeschlimann 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 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.