about quality assistant

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

Re: about quality assistant

Peter Uhnak
#beSomething is quite common pattern (even recommended in Kent's Best Practice Patterns), maybe it should check for name 'be'<Uppercase> before applying the critic?
This is a good proposal. Would be nice to hear other opinions. Maybe we can open an issue on the bug tracker so people can comment.

However I do not really want to ban the critic, because there may other legitimate applications in the method/class; and in Roassal-heavy code it's a nuisance.
So it would require context-aware checks (probably user-defined?) like "if receiver is subclass of RTShape then don't apply this (yourself) rule”.
 
Yes, it’s a very good point. It is in my plans to see how we can allow frameworks to provide their own critics and interact/override the default ones. For now all I can suggest is to ban the critic on the level of the class, or if we will reach the common sense there can be a ban on the level of package tag, so you can keep all the roassal-related classes in one tag and ban the rule for them.

I created issues for both so it doesn't get lost here.

Peter 


On Tue, Jul 28, 2015 at 3:24 PM, stepharo <[hidden email]> wrote:


Le 28/7/15 11:43, Alexandre Bergel a écrit :
But, what does the useless button do? Does it remove from the rules quality  assistant uses forever? Or is it per class? the rule "Too many methods" is useless for the class RTexample, but it is useful for other classes

Alex
I was confused too :)
I guess that it does nothing. Just report to the server of yuriy that you think that this rule is useless.
So there is a clear problem at the UI level. The user cannot disntinguish what he is doing:
    marking false positive
    or giving feedback in general.

Stef

Alexandre



Le 27 juil. 2015 à 12:53, Yuriy Tymchuk <[hidden email]> a écrit :

This is sad, and shows that I completely failed with that button. Because ideally you had to press it so I can know that this rule had a useful case for somebody.




Reply | Threaded
Open this post in threaded view
|

Re: about quality assistant

Peter Uhnak
I think that there is real problem with noise-to-signal ratio.
If I see four critics out of which tree makes in the context of the method/class no sense whatever then the result will not be that I improve my code, because I can't (those critics are false positives), but the result will be that I will start ignoring the critics altogether, because the ratio is too high.

This of course depends a lot on the code... if you need a lot of metaprogramming, or a library has some specific method naming (to be easier to use by hand) or whatever then this is a real problem.

Peter

On Tue, Jul 28, 2015 at 3:55 PM, Peter Uhnák <[hidden email]> wrote:
#beSomething is quite common pattern (even recommended in Kent's Best Practice Patterns), maybe it should check for name 'be'<Uppercase> before applying the critic?
This is a good proposal. Would be nice to hear other opinions. Maybe we can open an issue on the bug tracker so people can comment.

However I do not really want to ban the critic, because there may other legitimate applications in the method/class; and in Roassal-heavy code it's a nuisance.
So it would require context-aware checks (probably user-defined?) like "if receiver is subclass of RTShape then don't apply this (yourself) rule”.
 
Yes, it’s a very good point. It is in my plans to see how we can allow frameworks to provide their own critics and interact/override the default ones. For now all I can suggest is to ban the critic on the level of the class, or if we will reach the common sense there can be a ban on the level of package tag, so you can keep all the roassal-related classes in one tag and ban the rule for them.

I created issues for both so it doesn't get lost here.

Peter 


On Tue, Jul 28, 2015 at 3:24 PM, stepharo <[hidden email]> wrote:


Le 28/7/15 11:43, Alexandre Bergel a écrit :
But, what does the useless button do? Does it remove from the rules quality  assistant uses forever? Or is it per class? the rule "Too many methods" is useless for the class RTexample, but it is useful for other classes

Alex
I was confused too :)
I guess that it does nothing. Just report to the server of yuriy that you think that this rule is useless.
So there is a clear problem at the UI level. The user cannot disntinguish what he is doing:
    marking false positive
    or giving feedback in general.

Stef

Alexandre



Le 27 juil. 2015 à 12:53, Yuriy Tymchuk <[hidden email]> a écrit :

This is sad, and shows that I completely failed with that button. Because ideally you had to press it so I can know that this rule had a useful case for somebody.





Reply | Threaded
Open this post in threaded view
|

Re: about quality assistant

Uko2
I know that, and this is actually an issue that every system is facing. It is the next step to work on in QualityAssistant. One thing that I wander is whether there are people who want to keep noise. I.e. is it enough to simply cut off the non-accurate rules, people want to choose a threshold for the noise…

Uko

On 03 Aug 2015, at 15:08, Peter Uhnák <[hidden email]> wrote:

I think that there is real problem with noise-to-signal ratio.
If I see four critics out of which tree makes in the context of the method/class no sense whatever then the result will not be that I improve my code, because I can't (those critics are false positives), but the result will be that I will start ignoring the critics altogether, because the ratio is too high.

This of course depends a lot on the code... if you need a lot of metaprogramming, or a library has some specific method naming (to be easier to use by hand) or whatever then this is a real problem.

Peter

On Tue, Jul 28, 2015 at 3:55 PM, Peter Uhnák <[hidden email]> wrote:
#beSomething is quite common pattern (even recommended in Kent's Best Practice Patterns), maybe it should check for name 'be'<Uppercase> before applying the critic?
This is a good proposal. Would be nice to hear other opinions. Maybe we can open an issue on the bug tracker so people can comment.

However I do not really want to ban the critic, because there may other legitimate applications in the method/class; and in Roassal-heavy code it's a nuisance.
So it would require context-aware checks (probably user-defined?) like "if receiver is subclass of RTShape then don't apply this (yourself) rule”.
 
Yes, it’s a very good point. It is in my plans to see how we can allow frameworks to provide their own critics and interact/override the default ones. For now all I can suggest is to ban the critic on the level of the class, or if we will reach the common sense there can be a ban on the level of package tag, so you can keep all the roassal-related classes in one tag and ban the rule for them.

I created issues for both so it doesn't get lost here.

Peter 


On Tue, Jul 28, 2015 at 3:24 PM, stepharo <[hidden email]> wrote:


Le 28/7/15 11:43, Alexandre Bergel a écrit :
But, what does the useless button do? Does it remove from the rules quality  assistant uses forever? Or is it per class? the rule "Too many methods" is useless for the class RTexample, but it is useful for other classes

Alex
I was confused too :)
I guess that it does nothing. Just report to the server of yuriy that you think that this rule is useless.
So there is a clear problem at the UI level. The user cannot disntinguish what he is doing:
    marking false positive
    or giving feedback in general.

Stef

Alexandre



Le 27 juil. 2015 à 12:53, Yuriy Tymchuk <[hidden email]> a écrit :

This is sad, and shows that I completely failed with that button. Because ideally you had to press it so I can know that this rule had a useful case for somebody.






12