isKindOf considered questionable

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

isKindOf considered questionable

Stan Shepherd
Hi, I'm using isKindOf as part of my unit tests,

self should: [(mare maternalStrand at: 100)
                                                                isKindOf: Gene];

When I run through the code critics, isKindOf is flagged as a 'questionable message'. I can't find any reference to why it should be so; can someone explain please?

Thanks,   ...Stan
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf considered questionable

Tapple Gao
On Sat, Jun 21, 2008 at 08:13:48AM -0700, stan shepherd wrote:
>
> Hi, I'm using isKindOf as part of my unit tests,
>
> self should: [(mare maternalStrand at: 100)
> isKindOf: Gene];
>
> When I run through the code critics, isKindOf is flagged as a 'questionable
> message'. I can't find any reference to why it should be so; can someone
> explain please?

It is considered bad when isKindOf: is used in the following
way:

(anObject isKindOf: String) ifTrue: [anObject size]

It is good design to let an object decide what to do when it
recieves a message, rather than using isKindOf: to decide
whether you should send an object one message and not another.
The reason being that, someone may want to use a different kind
of object that respects the String protocol.

Objects are better classified by what messages they respond to,
not what class they may be.

For unit tests, this is a good use for it, though.

--
Matthew Fulmer -- http://mtfulmer.wordpress.com/
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: isKindOf considered questionable

Stan Shepherd

matthewf wrote
On Sat, Jun 21, 2008 at 08:13:48AM -0700, stan shepherd wrote:
>
> Hi, I'm using isKindOf as part of my unit tests,
>
> self should: [(mare maternalStrand at: 100)
> isKindOf: Gene];
>
> When I run through the code critics, isKindOf is flagged as a 'questionable
> message'. I can't find any reference to why it should be so; can someone
> explain please?

It is considered bad when isKindOf: is used in the following
way:

(anObject isKindOf: String) ifTrue: [anObject size]

It is good design to let an object decide what to do when it
recieves a message, rather than using isKindOf: to decide
whether you should send an object one message and not another.
The reason being that, someone may want to use a different kind
of object that respects the String protocol.

Objects are better classified by what messages they respond to,
not what class they may be.

For unit tests, this is a good use for it, though.

--
Matthew Fulmer -- http://mtfulmer.wordpress.com/
Thanks,   that makes sense.
...Stan
Reply | Threaded
Open this post in threaded view
|

code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Stan Shepherd
Quoting stan shepherd <[hidden email]>:

>

> >
> > It is considered bad when isKindOf: is used in the following
> > way:
> >
> > (anObject isKindOf: String) ifTrue: [anObject size]
> >
> > It is good design to let an object decide what to do when it
> > recieves a message, rather than using isKindOf: to decide
> > whether you should send an object one message and not another.
> > The reason being that, someone may want to use a different kind
> > of object that respects the String protocol.
> >
> > Objects are better classified by what messages they respond to,
> > not what class they may be.
> >
> > For unit tests, this is a good use for it, though.

>

Once you have established that the signaled message is not in fact an error, is
it possible to tell code critics not to signal that message for that code in
subsequent runs?

...Stan

_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Lukas Renggli
>  Once you have established that the signaled message is not in fact an error, is
>  it possible to tell code critics not to signal that message for that code in
>  subsequent runs?

Not out of the box. A long time ago I've written an extension that
allows to use method-annotations (pragmas) to do that. You can load
the code from <http://source.lukas-renggli.ch/essential.html>. You
find minimal documentation on that page as well.

If there is interest, I could include the basic functionality into RB
itself. I think that would be much cleaner and easier to use.

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Stan Shepherd
Quoting Lukas Renggli <[hidden email]>:

>
> Not out of the box. A long time ago I've written an extension that
> allows to use method-annotations (pragmas) to do that. You can load
> the code from <http://source.lukas-renggli.ch/essential.html>. You
> find minimal documentation on that page as well.
>
> Lukas
>
>

Thanks Lukas,

I'm getting an error on the URL http://source.lukas-renggli.ch/essential

'There has been an internal error. The system administrator has been notified'.

The repository doesn't then show up in Monticello.

...Stan
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Lukas Renggli
Use this url, with .html at the end:

    http://source.lukas-renggli.ch/essential.html

Lukas

On 6/23/08, [hidden email] <[hidden email]> wrote:

> Quoting Lukas Renggli <[hidden email]>:
>
>  >
>  > Not out of the box. A long time ago I've written an extension that
>  > allows to use method-annotations (pragmas) to do that. You can load
>  > the code from <http://source.lukas-renggli.ch/essential.html>. You
>  > find minimal documentation on that page as well.
>  >
>
> > Lukas
>  >
>  >
>
>  Thanks Lukas,
>
>  I'm getting an error on the URL http://source.lukas-renggli.ch/essential
>
>  'There has been an internal error. The system administrator has been notified'.
>
>  The repository doesn't then show up in Monticello.
>
>
>  ...Stan
>  _______________________________________________
>  Beginners mailing list
>  [hidden email]
>  http://lists.squeakfoundation.org/mailman/listinfo/beginners
>


--
Lukas Renggli
http://www.lukas-renggli.ch
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Stan Shepherd
Quoting Lukas Renggli <[hidden email]>:

> Use this url, with .html at the end:
>
>     http://source.lukas-renggli.ch/essential.html
>
> Lukas
>

> >  >
> >
> >  Thanks Lukas,
> >
> >  I'm getting an error on the URL http://source.lukas-renggli.ch/essential
> >
> >  'There has been an internal error. The system administrator has been
> notified'.
> >
> >  The repository doesn't then show up in Monticello.
> >
>

With:

MCHttpRepository
        location: 'http://source.lukas-renggli.ch/essential.html'
        user: ''
        password: ''

Transcript shows: 'redirecting to /@Rp3_OTsQFp6cJCVr/SrBb63Bv', then I get a
popup dialog with: 'Could not resolve the server named: '

sorry if I'm being dense,    ...Stan
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Lukas Renggli
>  Transcript shows: 'redirecting to /@Rp3_OTsQFp6cJCVr/SrBb63Bv', then I get a
>  popup dialog with: 'Could not resolve the server named: '

Indeed, there is a problem there. Sorry about that. I will check.

Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Lukas Renggli
> >  Transcript shows: 'redirecting to /@Rp3_OTsQFp6cJCVr/SrBb63Bv', then I get a
>  >  popup dialog with: 'Could not resolve the server named: '
>
> Indeed, there is a problem there. Sorry about that. I will check.

Ok, it should work again. There was something wrong with the file-system access.

Sorry for the inconvenience.

Cheers,
Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: Loading Essential package from Monticello (was isKindOf considered questionable)

Stan Shepherd
Quoting Lukas Renggli <[hidden email]>:

> > >  Transcript shows: 'redirecting to /@Rp3_OTsQFp6cJCVr/SrBb63Bv', then I
> get a
> >  >  popup dialog with: 'Could not resolve the server named: '
> >
> > Indeed, there is a problem there. Sorry about that. I will check.
>
> Ok, it should work again. There was something wrong with the file-system
> access.
>
> Cheers,
> Lukas
>

Hi Lukas,

With the .html suffix, it now opens the repository, but Transcript shows: 'Could
not read Essential-lr.25.mcz'

This happens after it looks in the cache. Here's the sequence of events:


versionInfo

        ^ selectedVersionInfo realizeFrom: repository

... where repository is a
MCHttpRepository(http://source.lukas-renggli.ch/essential)

realizeFrom: aRepository

        ^ aRepository realizeFullVersionInfo: self


realizeFullVersionInfo: aStubVersionInfoInstance

        ^ self versionInfoFromFileNamed: aStubVersionInfoInstance name


...where aStubVersionInfoInstance is
'Essential-lr.25.mcz'


versionInfoFromFileNamed: aString
        self cache at: aString ifPresent: [:v | ^ v info].
        ^ self loadVersionInfoFromFileNamed: aString


loadVersionInfoFromFileNamed: aString
        ^ [ self versionReaderForFileNamed: aString do: [:r | r info]
          ] on: Error
                        do: [ :ex | Transcript cr; show: 'Could not read ', aString.
                ].


versionReaderForFileNamed: aString do: aBlock
        ^ (self versionReaderForFileNamed: aString) ifNotNilDo: aBlock


loadVersionInfoFromFileNamed: aString
        ^ [ self versionReaderForFileNamed: aString do: [:r | r info]
          ] on: Error
                        do: [ :ex | Transcript cr; show: 'Could not read ', aString.
                ].


info
        info ifNil: [self loadVersionInfo].
        ^ info


loadVersionInfo
        info := self extractInfoFrom: (self parseMember: 'version')



loadVersionInfoFromFileNamed: aString
        ^ [ self versionReaderForFileNamed: aString do: [:r | r info]
          ] on: Error
                        do: [ :ex | Transcript cr; show: 'Could not read ', aString.
                ].



self versionReaderForFileNamed: 'Essential-lr.25.mcz'

returns a a MCMczReader with a ZipArchive whose members are an
OrderedCollection().

Let me know if I can provide more detail,   ...Stan
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Lukas Renggli
In reply to this post by Lukas Renggli
>  If there is interest, I could include the basic functionality into RB
>  itself. I think that would be much cleaner and easier to use.

The latest RB (Refactoring-Core-lr.5.mcz) integrates the functionality
to ignore certain SmallLint rules in particular methods. Contrary to
the original RB implementation it uses pragmas that are persistent
with the method. The original RB used a global dictionary that was not
tracked by versioning systems.

If the rule 'Messages sent but not implemented' should be ignored for
a particular method add the following pragma:

        <lint: 'Messages sent but not implemented'>

Normally you also want to add some more information about your rationale:

        <lint: 'Messages sent but not implemented' rationale: 'We are doing
metaprogramming trickery here' author: 'Lukas Renggli'>

Contrary to my original implementation in the Essential package don't
put a symbol to identify the rule (that doesn't work without changing
a whole lot of RB), but just the title that also appears in the result
browsers.

Furthermore I added several new rules to Lint: This is to detect not
categorized methods, violations to the Law of Demeter and
unconditional recursion. These rules were also formerly part of the
Essential package and can now be used from the standard RB tools.

Cheers,
Lukas

--
Lukas Renggli
http://www.lukas-renggli.ch
_______________________________________________
Beginners mailing list
[hidden email]
http://lists.squeakfoundation.org/mailman/listinfo/beginners
Reply | Threaded
Open this post in threaded view
|

Re: code Critics 'don't check again' optiion? (was isKindOf considered questionable)

Stan Shepherd

Lukas Renggli wrote
>  If there is interest, I could include the basic functionality into RB
>  itself. I think that would be much cleaner and easier to use.

The latest RB (Refactoring-Core-lr.5.mcz) integrates the functionality
to ignore certain SmallLint rules in particular methods.
That sounds really good, and I will definitely check it out. Will this automatically feed into the the development packages?

...Stan