Re: [Scaffoldhunter-devel] Fwd: Re: Random Subset Patch File version 2

[Till S. <til...@tu...>]

Hi, 
lets keep the open source spirit and discuss this over the devel mailing list ;-)

Am Montag, 7. April 2014, 17:19:42 schrieb Andrew Zhilka:
> Hi Till,
> 
> The new-size field already accepts only numbers but I will also add a label. 
I meant something different. Currently it is possible to type non numeric letters, but the ok button is only active if the text field contains a number.  However it is also possible to restrict the text field in such a way, that pressing a non-numerical char results in no change of the text field. 

One further suggestion (i do not expect you to do this, but if you have fun to do so this would make the dialog perfect for me :-) ):
I just noticed, that you also check the size of the parent subset. This is a nice feature. A small problem that remains, is the the user might be unaware about the cause of the disabled ok button. We should indicate the cause in some way.  The easiest would be to show a message like "value to large" or "cannot create a subset with a size larger than than the parent subset size". The problem is, that we have a some of empty space (if no message ist shown) or the dialog changes the size. An alternative would be to show a tool tip over text field that explains the error, but the user might miss the tool tip. We can also show small info icon (behind the text field / if the size error occurs) that shows the error message as tool tip and surround the text field with a red line like in the "new user dialog".   What do you think is the best solution?

> Thanks for letting me know about SwapArrayList, I shall make use of it.;)
Thank you, too.

Greetings 
Till 


> 
> Regards,
> Andrew "Anjenson" Zhilka
> 
> -----Original Message-----
> From: "Till Schäfer" <til...@tu...>
> Sent: ‎07/‎04/‎2014 14:32
> To: "hot...@gm..." <hot...@gm...>
> Cc: "sca...@li..." <sca...@li...>
> Subject: Re: Fwd: Re: Random Subset Patch File version 2
> 
> Hi Andrew, 
> thank a lot for submitting the patch! As you may have noticed, nils already committed it in the trunk.  I have looked through the code / usability and have two further suggestions:
> 
> 1. Performance: 
> The current implementation has a quadratic runtime behavior. The single operations are fast, but it can take some seconds for a larger dataset. This is caused by the removal from the ArrayList which takes O(n) / linear time in the worst and average case. You can avoid this overhead by swapping the element which should be removed with the last element in the list and remove the last element afterwards (since you do not depend on the ordering of the list). For this purpose i already implemented a SwappingArrayList and i just committed it to the trunk (see the util package). That means you can just call the swapAndRemove method. 
> 
> 2. The user interface: 
> In my opinion we should not use two dialogs that pop up after each other. Two dialogs do fragment the workflow.  Furthermore, i can not read the complete title of the window in my window manager as there is not enough space. Therefore, the purpose of the text field has to be guessed (there are also window managers out there with no title at all ;) ).
> -> We should create a label in front of text field that explains the dialog (something like: "please enter the size of the random subset"). I know the already implemented rename dialog was not the best example here ;) 
> -> we should remove the second dialog and integrate the naming directly in the first dialog OR just create the subset with a fixed name (the user can rename it afterwards if the name does not fit. together with the below suggestion about the default naming, this seem a solutions that is not less usable than the first one / you can decide here)
> -> the size field should only accept numbers (we do not need the explanation in the field anymore if we have a label)
> -> we should give a more meaningful default name: e.g.: random(parent name, size). That means the user can directly see that this subset is created randomly from parent subset with a specific size).
> 
> This were more suggestions than i intended at the beginning of this mail ;-) Nevertheless, this are the usual things that come up after patch submission. 
> 
> Greetings
> Till
> 
> Am Montag, 7. April 2014, 11:53:51 schrieb Nils Kriege:
> > 
> > ----------  Forwarded Message  ----------
> > 
> > Subject: Re: Random Subset Patch File version 2
> > Date: Monday 07 April 2014, 10:40:02
> > From: Nils Kriege <nil...@tu...>
> > To: Anjenson <hot...@gm...>
> > 
> > Hi Andrew,
> > 
> > thank you for your work on this feature. Your code looks fine and I have 
> > comitted it to the repository.
> > 
> > 
> > Thanks!
> >   Nils
> > 
> > On Friday 04 April 2014 20:02:05 Anjenson wrote:
> > > Hi Nils,
> > > 
> > > Sending you fixed version.
> > > 
> > > Regards,
> > > Andrew
> > -----------------------------------------
> 
-- 
Dipl.-Inf. Till Schäfer
TU Dortmund University
Chair 11 - Algorithm Engineering
Otto-Hahn-Str. 14 / Room 237
44227 Dortmund, Germany

e-mail: til...@cs...
phone: +49(231)755-7706
fax: +49(231)755-7740
web: http://ls11-www.cs.uni-dortmund.de/staff/schaefer
pgp: https://keyserver2.pgp.com/vkd/SubmitSearch.event?&&SearchCriteria=0xD84DED79