a lot of equations like type(x) == type("")

[getxsick]

there are thousands of them in umit code. even better is type(x) is object (e.g. type(x) is str but perhaps better is to use isinstance() than checking about.

another issue is that type checking in general meaning about bad style ;)

[gpolo]

The idiom "type(x) is str" is actually worse than "type(x) == type()" and equivalents because you can shadow the str type by an arbitrary object. And it is very likely that you want/need to check against both str and unicode in several places in umit (but this is not really related to this ticket).

I prefer isinstance over type calls, specially because there is this umitplugins now.

Finally, as your last phrase says, it is very possible that several of these checks should go in order to not hide other bugs (I noticed most of them could go in the NmapParser?, but didn't remove yet).

[gpolo]

trac ate my second example:

type(x) == type('')

[getxsick]

so are we going to do anything with this issue or leave as it is?

[gpolo]

Each case, or a group of them, need to be evaluated separately. If you have any patches over there, just attach them here.

[getxsick]

I attached a patch which replace all type checking in whole umit by instance checking. Propably some works are still needed for avoid useless checkings anyway.

[nopper]

Replace isinstance(object, str) with isinstance(object, basestring) and then you could commit to trunk imho.

[getxsick]

updated. i'm waiting for gpolo vote, cause he was active in this ticket...

[gpolo]

That is very far from being a patch that "replaces all type checking in while umit by instance checking", getxsick. umit/gui/SearchGUI has several other cases, as does umit/gui/OptionBuilder

Also, what are we gaining by blindly changing type calls to isinstane calls ? Take this function "switch_host_details" in umit/gui/ScanNotebook, why does it have to do such type check ? It would be way better if type checks were eliminated, instead of just replaced by isinstance calls. Apparently I wasn't clear enough when I said the cases needed to be evaluated.

[getxsick]

Replying to gpolo:

That is very far from being a patch that "replaces all type checking in while umit by instance checking", getxsick. umit/gui/SearchGUI has several other cases, as does umit/gui/OptionBuilder

ok, i made it to fast and forgot about those where just create new instance i'm attaching a new patch, also i removed deprecated StringTypes?

Also, what are we gaining by blindly changing type calls to isinstane calls ? Take this function "switch_host_details" in umit/gui/ScanNotebook, why does it have to do such type check ? It would be way better if type checks were eliminated, instead of just replaced by isinstance calls. Apparently I wasn't clear enough when I said the cases needed to be evaluated.

isn't the same what i said when i created a ticket and in the comment when attached the file?

Propably some works are still needed for avoid useless checkings anyway.

So yes, it doesn't mean that I'm going to close this ticket. It's just a some simple (or blindly if you would preffer) changing, which are make code even better than before, but some other works (which we both pointed) should be done too.

[getxsick]

removing StringTypes? and type(X()) style

[getxsick]

Replying to gpolo:

Also, what are we gaining by blindly changing type calls to isinstane calls ?

I thought that i included answer for this in your second question but for clarify.

We are just making it in proper way, which is officialy recommended by Python Software Foundation. So it's some good point for the beginning, and next evolution should be done.

[ignotus]

I think getxsick can apply the patch (or an updated version of it, since is a year old patch) and open another ticket for the remaining "problems".

[getxsick]

Fixed in: r5563 r5564 r5565 r5566 r5567 r5568 r5569 r5570 r5571