SetNofTabs problem

[luowy]

ok, I have to recompile some modules which import it.
if you concern about the optimaztion,why ignore the improvement of SetNofTabs which I have done.
I think it is important when this view is heavily used as a docu browser.
and be careful when using SetNofTabs, a empty item have a side effect that items behind it will not work,
and no error message also.
example(pesudo code):

Code: Select all

SetNofTabs(4)
SetItem(0) 
SetItem(2) 
SetItem(2) 
SetItem(3) 
Views.Open()

only one tab view do work.

[Josef Templ]

SetItem is no need changed, in my opinion.

This change is disputable indeed. May be I should not have done it.
I had the feeling that most center members prefer to use IN
if it can be applied easily. Sorry for any inconveniences but we are in
alpha state, so there can always be the need to recompile client code.
Using IN does not complicate the coding, so it is really simple compared
to your SetNofTabs optimization, which introduces additional code and leaves back
an array that is actually oversized. It is not possible to imagine a use case where
this optimization brings an observable advantage.

SetNofTabs(4)
SetItem(0)
SetItem(2)
SetItem(2)
SetItem(3)
Views.Open()

I don't understand this test case. Can you please give us a compilable version and more details.

- Josef

[luowy]

test this code

Code: Select all

	
	IMPORT Views, StdTabViews, TextViews;

	VAR tv: StdTabViews.View;
	
PROCEDURE Open*;  
	BEGIN
		tv := StdTabViews.dir.New();
		tv.SetNofTabs(4); 
		tv.SetItem(0, "x", TextViews.dir.New(NIL));
		tv.SetItem(2, "xx", TextViews.dir.New(NIL));
		tv.SetItem(2, "xxx", TextViews.dir.New(NIL));
		tv.SetItem(3, "xxxx",TextViews.dir.New(NIL));
		Views.OpenAux(tv, "Test");
	END Open;
	

[Robert]

I am guessing this example only generates 1 Tab ?

I don't think that is a bug. You are not allowed to explicitly set empty Tabs (with SetItem), so why should you be allowed to do it implicitly (new heap strings are initialised to 0X)?


And "Yes", I do prefer "IN".

[Josef Templ]

we could introduce empty labels as a marker for a tab that is not displayed.
This could be used for switching tabs on or off without terminating the list of tabs.
Don't know how much change this is in HostTabFrames.

- Josef

[Ivan Denisov]

if you concern about the optimaztion,why ignore the improvement of SetNofTabs which I have done.

luowy, can you, please, review is your optimizations included in last unstable version?
Did you post them here in the board or they are included in CPC version?

[luowy]

Code: Select all

	PROCEDURE (tv: View) SetNofTabs* (nofTabs: INTEGER), NEW;
		VAR i: INTEGER; nt: Tabs;
	BEGIN
		ASSERT(nofTabs >= 0, 20);
		IF nofTabs = 0 THEN
			tv.nofTabs := 0; tv.tabs := NIL; tv.index := 0
		ELSE
			IF (nofTabs <= tv.nofTabs)&(tv.tabs # NIL) THEN 
				FOR i :=nofTabs TO tv.nofTabs-1 DO tv.tabs[i].label:=''; tv.tabs[i].view:=NIL; END;
			ELSE 
				NEW(nt, nofTabs);
				IF tv.tabs # NIL THEN
					FOR i := 0 TO MIN(nofTabs, tv.nofTabs) - 1 DO
						nt[i].label := tv.tabs[i].label; nt[i].view := tv.tabs[i].view;  (*nt[i].g:=NIL;*)
					END
				END;
				tv.tabs := nt; 
			END;
			tv.nofTabs := nofTabs; tv.index := MIN(tv.index, tv.nofTabs - 1);
		END;
		tv.Update
	END SetNofTabs;

[Josef Templ]

See http://forum.blackboxframework.org/view ... Tabs#p2966.

This is an irrelevant optimization and actually introduces several drawbacks.
It makes the garbage collection performance worse, it wastes memory and in extreme cases it would
be slower than the unoptimized case.

Consider the case where you have a large number of tabs, say 1000, and then set the new number of tabs to 1,
the optimization would eliminate one single NEW. In exchange for that it has to set 999 entries to NIL.
From that time on the garbage collector has to visit 999 unused array entries to find out that they are NIL.
In addition, the array uses much more space than with the unoptimized case.
(In addition, only the case of shrinking the number of tabs is optimized but the same optimization
could be applied to enlarging it if there is space left in the array.)

- Josef

[luowy]

See http://forum.blackboxframework.org/view ... Tabs#p2966.

This is an irrelevant optimization and actually introduces several drawbacks.
It makes the garbage collection performance worse, it wastes memory and in extreme cases it would
be slower than the unoptimized case.

Consider the case where you have a large number of tabs, say 1000, and then set the new number of tabs to 1,
the optimization would eliminate one single NEW. In exchange for that it has to set 999 entries to NIL.
From that time on the garbage collector has to visit 999 unused array entries to find out that they are NIL.
In addition, the array uses much more space than with the unoptimized case.
(In addition, only the case of shrinking the number of tabs is optimized but the same optimization
could be applied to enlarging it if there is space left in the array.)

- Josef

I agree,no need at all,just for Ivan's requset

[Ivan Denisov]

Thank you LuoWy and Josef. I am trying to make the board more ordered before stable release.