issue-#111 code cleanups

Merged to the master branch
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

issue-#111 code cleanups

Post by Josef Templ »

This issue refers to code cleanups as proposed in CPC 1.7 rc6 (changes by Trurl).
As far as I see this would be the last issue from the CPC change list.

For the issue see http://redmine.blackboxframework.org/issues/111.

I will not go over all modules but touch only those listed in the CPC change log.
Any objections?

- Josef
User avatar
Robert
Posts: 1024
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: issue-#111 code cleanups

Post by Robert »

I've forgotten where the CPC change log file is kept!
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#111 code cleanups

Post by Ivan Denisov »

Josef, then you will do any manipulation, please, be sure, that at the end you set default font typeface for sources. Not Arial.
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#111 code cleanups

Post by Josef Templ »

here are the changes: http://redmine.blackboxframework.org/pr ... ab5d966240.

Please review carefully.
I didn't change any font but also didn't check if all are OK.

I have not done any changes of WriteSString to WriteString.
I don't see any advantage in this change unless required for UNicode support.

- Josef
User avatar
DGDanforth
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA
Contact:

Re: issue-#111 code cleanups

Post by DGDanforth »

Josef,
There are other modules that need cleaning up.
I'll post a list later.
-Doug
User avatar
DGDanforth
Posts: 1061
Joined: Tue Sep 17, 2013 1:16 am
Location: Palo Alto, California, USA
Contact:

Re: issue-#111 code cleanups

Post by DGDanforth »

Josef,
Here is the list of modules I found using GftSearchFiles looking for "^".
Not all such instances are redundant since forward procedures use that symbol and
SYSTEM.ADR must act on the actual data and not the pointer to the data, etc.

Within each file I give only a hint of the lines needing cleanup so there may be more lines
in a module than are indicated in the list below. I believe all modules have been correctly identified.
-Doug

Code: Select all

Comm/Mod/TCP.odc
	inaddr := SYSTEM.VAL(WinNet.in_addr, hostentry.h_addr_list^[0]^)
Ctl/Mod/C.odc
	SYSTEM.MOVE(a, SYSTEM.ADR(x^), LEN(x^) * SIZE(BYTE));
	and other such references
Dev/Mod/Markers.odc
	NEW(v.msg, LEN(source.msg^)); v.msg^ := source.msg^$
		f.DrawString(2 * point, asc, color, v.msg^, font)
		etc
Host/Mod/Bitmaps.odc
	rd.rider.ReadBytes(data^, 0, len);
Host/Mod/CFrames.odc
	possible unnecessary "^"
Host/Mod/Pictures.odc
		possible unnecessary "^"
Obx/Mod/Excel.odc
		possible unnecessary "^"
Obx/Mod/Fact.odc
		possible unnecessary "^"
Obx/Mod/Ratcalc.odc
			possible unnecessary "^"
Sql/Mod/DB.odc
	ELSIF t.idx + max >= LEN(t.ext^) THEN
	etc
Sql/Mod/ObxDriv.odc
	IF (str = NIL) OR (<length of string (incl 0X)> > LEN(str^)) THEN
	etc
Sql/Mod/Odbc.odc
	IF (str = NIL) OR (LEN(str^) < LEN(s)) THEN NEW(str, LEN(s)) END;
		etc
Sql/Mod/Odbc3.odc
		IF (str = NIL) OR (LEN(str^) < LEN(s)) THEN NEW(str, LEN(s)) END;
		etc
Std/Mod/Dialog.odc
	GetGuardProc(i.filter^, x, p, n);
	etc.
Std/Mod/Links.odc
	IF v.cmd # NIL THEN Reveal(left, right, v.cmd^, "#StdLinks:Reveal Link Command") 
	etc
Std/Mod/Loader.odc
	inp.ReadBytes(mp^, 0, mod.ms);
	etc
Std/Mod/Stamps.odc
	op.stamp.comment0 := op.oldcomment^$
	v.history[i].comment^ := source.history[i].comment^$;
	f.WriteString( v.history[i].comment^);
	IF v.history[entryno].comment # NIL THEN comment := v.history[entryno].comment^$
System/Mod/Dialog.odc
	IF tab.key = NIL THEN j := 0 ELSE j := LEN(tab.key^) END;
	ELSIF LEN(s.sel^) # len THEN
	IF LEN(s.sel^) < len THEN len := LEN(s.sel^) END;
System/Mod/Integers.odc
	y[0] := 1; yL := 1; NEW(z, LEN(y^));
	x[LEN(x^) - 1] := low; x[LEN(x^) - 2] := high
	low := x[LEN(x^) - 1]; high := x[LEN(x^) - 2];
System/Mod/Kernel.odc
	Utf8ToString(p^$, name, res); ASSERT(res = 0)
System/Mod/Meta.odc
	IF (i < n) & (i < LEN(x)) THEN x := p^$; ok := TRUE
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#111 code cleanups

Post by Josef Templ »

Going over all modules is too much work and there is alway the danger of
introducing a nasty typo (not detected by the compiler!!) in any change we make.
There are tons of possible improvements, not just the ^ and it is not always clear
if something is an improvement or not, so we will end up in endless discussions.

I would propose to stop now with the cleanups.
The only reason why I did them was to be in line with the CPC changes.
The set of changed modules does not express any personal preference from my side.

- Josef
Zinn
Posts: 476
Joined: Tue Mar 25, 2014 5:56 pm
Location: Frankfurt am Main
Contact:

Re: issue-#111 code cleanups

Post by Zinn »

I agree to stop this kind of changes.
After we have vote over the open issues and they merge to the master I will create a new version of the CPC Edition.
Then we can discuss the differences between the Center and the CPC Edition and minimize the differences.
Open issues are #100, #106, #107, #108 and #111 as far as I know.

- Helmut
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: issue-#111 code cleanups

Post by Ivan Denisov »

Josef and Helmut, so we can create the voting already?
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: issue-#111 code cleanups

Post by Josef Templ »

Looking at StdLog I found a strange syntax for some of the procedures.
An example is:

Code: Select all

	PROCEDURE* ClearBuf;
		VAR subBuf: TextModels.Model;
	BEGIN
		subBuf := subOut.rider.Base(); subBuf.Delete(0, subBuf.Length())
	END ClearBuf;
Note the star after PROCEDURE.
This is an undocumented feature of the compiler.
The star is silently ignored.
I cannot see any special case here.
It is simply a global unexported procedure.

The options are:
1. remove the star in StdLog procedures. IMHO, should be done.
2. remove this undocumented feature from the compiler (DevCPP.ProcedureDeclaration)
Note: This will create problems in the build process for branches that still use the old syntax
and for source code that accidentally uses this feature or when re-compiling an older revision.
So I am not sure if this is a good idea.

- Josef
Post Reply