Ron Jeffries | 1 Oct 04:02 2004
Picon

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
(Continue reading)

Joshua Kerievsky | 4 Oct 07:50 2004

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);
(Continue reading)

Sven Gorts | 4 Oct 09:58 2004
Picon

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();
>     }
> 
(Continue reading)

Dagfinn Reiersøl | 4 Oct 11:05 2004
Picon
Picon

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:
(Continue reading)

Joshua Kerievsky | 4 Oct 18:21 2004

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

(Continue reading)

Thomas Eyde | 4 Oct 18:38 2004
Picon
Picon

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
(Continue reading)

Joshua Kerievsky | 4 Oct 18:22 2004

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
(Continue reading)

Picon

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();
>     }
> 
(Continue reading)

Keith Nicholas | 4 Oct 22:33 2004

RE: making some code more readable

I think your problems are due to lack of encapsulation.
 
your taking an object "String" and an object "Hashtable" and yet another object which has nothing to do with either of those that takes both objects and spits out another String object.
 
Message message = new message("Gidday ${firstName}");
message.replaceVariblesWith(values);
 
not sure about the HashMap as a store for your data but there are no other tests to indicate a better object at this stage.
 
Regards,
 
Keith
-----Original Message-----
From: Joshua Kerievsky [mailto:joshua <at> industriallogic.com]
Sent: Monday, 4 October 2004 6:50 p.m.
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();
    }

    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);

To create the more readable version, I moved the parameter,
templateText, from the method, replaceValue, to the constructor for
TemplateMessageBuilder (within which it is assigned to a field).  I
might call such a refactoring "Replace Parameter with Field" or, more
simply, "Move Parameter." 

However, I'm more interested in the code smell than the refactorings to
clean up this code.  So what odor do we smell here?   It certainly isn't
"Long Parameter List" -- since the list of parameters isn't long.   And,
IMHO, none of the other articulated smells really seem to fit.  So
perhaps this is a new smell.   The question is, what words can we use to
describe it?

The closest I can come is the term "Misplaced Data"    The data in
question ought to be field instead of a parameter or local variable.

Does that name speak to you?   Or is there a better phrase to describe
this smell?

best regards,
jk




Yahoo! Groups Links
J. B. Rainsberger | 5 Oct 01:08 2004

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/


Gmane