Re: duplication in method & parameter names

On Thursday, September 30, 2004, at 5:48:50 PM, J. B. Rainsberger wrote:

> Buddha Buck wrote:
>> You Smalltalkers might sell me on the idea of Smalltalk afterall.  For
>> some reason
>> 
>>   send thisEmail: confirmationEmail toServer: someServer
>> 
>> reads even better to me.
>> 
>> (Even so, that looks like it should read:   confirmationEmail sendTo:
>> someServer)

> I prefer this, if only because it's possible to send an e-mail to 
> something other than a server, and if that something-else knows how to 
> respond, then so much the better. There's an abstraction missing, and 
> someday it'll pop up, or we'll die -- whichever comes first. :)

The second form has the additional value that it is legal smalltalk, and
unless send is an object, the first one isn't ... :)

Ron Jeffries
www.XProgramming.com
Keep away from people who try to belittle your ambitions. Small people
always do that, but the really great make you feel that you, too, can
become great." -- Mark Twain.

------------------------ Yahoo! Groups Sponsor --------------------~--> 
$9.95 domain names from Yahoo!. Register anything.
http://us.click.yahoo.com/J8kdrA/y20IAA/yQLSAA/umvwlB/TM

making some code more readable

Folks,

I was looking at the test method, 
testVariableMarkerIsReplacedWithValue(), shown below:

public class TemplateMessageBuilderTest extends ShopTestCase...
    private TemplateMessageBuilder messageBuilder;

    public void setUp() {
        messageBuilder = new TemplateMessageBuilder();
    }

    public void testVariableMarkerIsReplacedWithValue() {
        String templateText = "Hello, ${firstName} How's the weather?";
        Map values = new HashMap();
        values.put("firstName", "Max");

        String result = messageBuilder.replaceValue(templateText, values);
        assertEquals("Hello, Max How's the weather?", result);
    }

Within that method, I don't like this line of code:

        String result = messageBuilder.replaceValue(templateText, values);

Why don't I like it?  Mostly because it doesn't read like English.  
Here's how I want the code to read:

        String result = messageBuilder.replaceVariablesWith(values);

Re: making some code more readable

Hi Josh,

  What do you think of the term "External Data" ?

  Rather than "Misplaced Data", "External Data" 
  expresses that the data is living somewhere 
  outside it's natural home.

Kind regards,
Sven

--- Joshua Kerievsky <joshua <at> industriallogic.com>
wrote:

> Folks,
> 
> I was looking at the test method, 
> testVariableMarkerIsReplacedWithValue(), shown
> below:
> 
> public class TemplateMessageBuilderTest extends
> ShopTestCase...
>     private TemplateMessageBuilder messageBuilder;
> 
>     public void setUp() {
>         messageBuilder = new
> TemplateMessageBuilder();
>     }
> 

Re: making some code more readable

Joshua Kerievsky wrote:

>Folks,
>
>I was looking at the test method,
>testVariableMarkerIsReplacedWithValue(), shown below:
>
>public class TemplateMessageBuilderTest extends ShopTestCase...
>    private TemplateMessageBuilder messageBuilder;
>
>    public void setUp() {
>        messageBuilder = new TemplateMessageBuilder();
>    }
>
>    public void testVariableMarkerIsReplacedWithValue() {
>        String templateText = "Hello, ${firstName} How's the weather?";
>        Map values = new HashMap();
>        values.put("firstName", "Max");
>
>        String result = messageBuilder.replaceValue(templateText, values);
>        assertEquals("Hello, Max How's the weather?", result);
>    }
>
>Within that method, I don't like this line of code:
>
>        String result = messageBuilder.replaceValue(templateText, values);
>
>Why don't I like it?  Mostly because it doesn't read like English. 
>Here's how I want the code to read:

Re: making some code more readable

Dagfinn Reiersøl wrote:

>I'm not saying this is better, but you could have changed the name and swapped the parameters:
>
>        String result = messageBuilder.replaceVariablesWith(values,templateText);
>
>Interpreted as English, this could read something like "replace variables with values in the template text".
>
>This indicates to me that the more important change may be changing the name of the method rather than
moving the parameter.
>  
>
I'd have to disagree, because to me, the tempate text is data that the 
object being called ought to already know about.   --jk

------------------------ Yahoo! Groups Sponsor --------------------~--> 
$9.95 domain names from Yahoo!. Register anything.
http://us.click.yahoo.com/J8kdrA/y20IAA/yQLSAA/umvwlB/TM
--------------------------------------------------------------------~-> 

 
Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/refactoring/

<*> To unsubscribe from this group, send an email to:
    refactoring-unsubscribe <at> yahoogroups.com

Re: making some code more readable

Dagfinn Reiersøl wrote:

>I'm not saying this is better, but you could have changed the name and swapped the parameters:
>
>        String result = messageBuilder.replaceVariablesWith(values,templateText);
>
>Interpreted as English, this could read something like "replace variables with values in the template text".
>
>This indicates to me that the more important change may be changing the name of the method rather than
moving the parameter.
>
>  
>
Following this path, we could rename the parameters and the method:
    string result = messageBuilder.replaceVariables(inTemplateText, 
withValues);
or
    string result = messageBuilder.replaceVariables(withValues);

Personally, I feel this is a little better:
    string result = messageBuilder.replacePlaceholders(inTemplateText, 
withValues);

Thomas

------------------------ Yahoo! Groups Sponsor --------------------~--> 
Make a clean sweep of pop-up ads. Yahoo! Companion Toolbar.
Now with Pop-Up Blocker. Get it for free!
http://us.click.yahoo.com/L5YrjA/eSIIAA/yQLSAA/umvwlB/TM

Re: making some code more readable

Sven Gorts wrote:

>Hi Josh,
>
>  What do you think of the term "External Data" ?
> 
>  Rather than "Misplaced Data", "External Data" 
>  expresses that the data is living somewhere 
>  outside it's natural home.
>  
>
Well, we could also apply the smell Misplaced Data to a case where some 
data is living it up as a field in a class when it really ought to be a 
lowly parameter.   External Data would only discuss the external case.   
--jk

------------------------ Yahoo! Groups Sponsor --------------------~--> 
$9.95 domain names from Yahoo!. Register anything.
http://us.click.yahoo.com/J8kdrA/y20IAA/yQLSAA/umvwlB/TM
--------------------------------------------------------------------~-> 

 
Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/refactoring/

<*> To unsubscribe from this group, send an email to:
    refactoring-unsubscribe <at> yahoogroups.com

RE: making some code more readable

Is this real code or just an example? 

If it is real then I would want to know what the intended purpose of TemplateMessageBuilder class is. Is
"replaceValue" all TemplateMessageBuilder does? Perhaps it should not even be a separate class. I don't
have enough information to know if you have identifed a valid smell or the right smell.

If it is an example of a general smell, then I want a better example. Given that you are manipulating a String I
am not sure I see a smell I would be inclined to worry about. 

> -----Original Message-----
> From: Joshua Kerievsky [mailto:joshua <at> industriallogic.com]
> Sent: Monday, October 04, 2004 12:50 AM
> To: refactoring <at> yahoogroups.com
> Subject: [refactoring] making some code more readable
> 
> 
> 
> Folks,
> 
> I was looking at the test method, 
> testVariableMarkerIsReplacedWithValue(), shown below:
> 
> public class TemplateMessageBuilderTest extends ShopTestCase...
>     private TemplateMessageBuilder messageBuilder;
> 
>     public void setUp() {
>         messageBuilder = new TemplateMessageBuilder();
>     }
> 

RE: making some code more readable

Re: making some code more readable

Joshua Kerievsky wrote:
> Dagfinn Reiersøl wrote:
> 
>  >I'm not saying this is better, but you could have changed the name and 
> swapped the parameters:
>  >
>  >        String result = 
> messageBuilder.replaceVariablesWith(values,templateText);
>  >
>  >Interpreted as English, this could read something like "replace 
> variables with values in the template text".
>  >
>  >This indicates to me that the more important change may be changing 
> the name of the method rather than moving the parameter.
>  > 
>  >

> I'd have to disagree, because to me, the tempate text is data that the
> object being called ought to already know about.   --jk

I think there's a more general refactoring here. Replace procedure with 
object. (??)

This:
procedure.doToWith(toMe, withMe)

Becomes:
new Thing(toMe).doWith(withMe)

This is an easy refactoring to motivate when a class has 5 methods that 
each take the same first parameter, but what if there is only one?
-- 
J. B. (Joe) Rainsberger
Diaspar Software Services
http://www.diasparsoftware.com :: +1 416 791-8603
Predictable, repeatable, quality delivery

------------------------ Yahoo! Groups Sponsor --------------------~--> 
$9.95 domain names from Yahoo!. Register anything.
http://us.click.yahoo.com/J8kdrA/y20IAA/yQLSAA/umvwlB/TM
--------------------------------------------------------------------~-> 

 
Yahoo! Groups Links

<*> To visit your group on the web, go to:
    http://groups.yahoo.com/group/refactoring/

<*> To unsubscribe from this group, send an email to:
    refactoring-unsubscribe <at> yahoogroups.com

<*> Your use of Yahoo! Groups is subject to:
    http://docs.yahoo.com/info/terms/