readability checker board

[BasicCoder2]

Wasn't sure if to let this one pass but I am a sucker for punishment !!
This is a new thread and probably in the wrong section but we were hijacking bluatigro thread.

Ok. STEP is "readable" once you get it and perhaps are cognisant of the hidden draw variables for position.

Code: Select all

screenres 640,480,32
pset (100,100)   'set draw coordinates at 100,100
line -(40,40)    'draw relative to 140,140
sleep

No, no, no --> draw 100,100 to 40,40. It only keeps the last position.

For testing try with

Code: Select all

screenres 640,480,32
pset (100,100)
line -(0,0)
sleep

Not sure why you thought I didn't know the variables held the last position.
You can return them with the overloaded point function.

Usually I would put readability (by a novice) before speed.

Your codes (all of them) are examples, of non-readability, IMHO.
(all the clumsy calculations, in Sub/Function parameters, to be precise,
apart from the fact, that it is also: sloooooooooow)

:)
I personally have no trouble reading them and I find them fast enough.
By "readability" I mean as readable as possible by someone who knows little if anything about programming languages.

[MrSwiss]

I personally have no trouble reading them and I find them fast enough.

This is, as you can clearly see, NOT the same as:

Usually I would put readability (by a novice) before speed.

IMHO, readable code also needs to be: "commentable", which in your style, is clearly close to impossible.

In your calculations (as parameters), a novice simply has to guess, what you're doing.

Just a code excerpt, to explain above better:

Code: Select all

For j As UInteger = 0 To 7
    Var v = 60 + j * 60         ' calc. vertical pos.
    For i As UInteger = 0 To 7
        Var h = 60 + i * 60     ' calc. horizontal pos.
        Line (v, h)-Step(sq, sq), c2, B
        cnt += 1
    Next
    cnt += 1
Next


[caseih]

Having looked at his checkers code, I would agree with BasicCoder2 that his code is quite readable as it is. Comments are important and enhance readability, but only if they are used where they are necessary and no more. They can help or hinder readability. Modern programming languages like FB are quite expressive and the code itself is a form of expression and can stand alone. It shouldn't be necessary to comment a for loop, for example. It is obvious what is happening to the counter variable. A general comment about what the loop is trying to accomplish is certainly desirable of course. Could the checkers example have a few more comments? Probably. I'm sceptical that BasicCoder2 would need to add very many comments, though, to make it more understandable. This is only my opinion, and it's shared by quite a few professional programmers. There are obviously differing opinions.

EDIT: I see your latest edit, MrSwiss, and yes a couple of comments like you have added there are appropriate. I wouldn't say one has to guess at the uncommented code, though. The fact that line is drawing from a set of coordinates makes it pretty clear those two lines are calculating position. It's not unreasonable for a programmer to assume future readers of the code know that, since LINE is a fundamental part of the language. So I could understand an argument either way on that. I don't think it's fundamentally important or necessary to think of a novice programmer reading the code when writing it (unfamiliar with LINE in this case). It's perfectly reasonable to assume (or demand) that whoever comes after me will be skilled in the art of programming.

[BasicCoder2]

By "readable by a novice" I meant as an ideal I might aim for. I added "novice" because of course an advanced programmer such as yourself can easily read code that might be cryptic to me or a beginner. Readability is programmer dependent. I meant I like to make it as simple and human language like where possible. That might mean hiding things like the cryptic LINE command in a function. When I make the effort to do a proper job I think you don't have to be an advanced programmer to read most of my code. I would add that explaining what the code does and how the calculations work can be external to the source code itself and may involve diagrams.

Code: Select all

''Draw an 8 x 8 array of square shapes 60 pixels apart with dimensions 30x30 pixels
screenres 640,600,32
sub drawSquare(x as integer, y as integer, s as integer, c as ulong)
  line (x,y)-(x+s,y+s),c,b
end sub

'calculate column position i by multipling by 60 pixels and adding 60 pixel displacement
'calculate row position j by multipling by 60 pixels and adding 60 pixel displacement
for j as integer = 0 to 7
  for i as integer = 0 to 7
    drawSquare(i*60+60,j*60+60,30,rgb(255,0,0)) 'drawSquare(column, row, size, color)
  next i
next j

sleep


With regards to taking the vertical and horizontal calculations out of the paramater list I think you are taking away something that I found great when I first understood that a variable was just a simple expression. I am rubbish at math but even I found that calculation easy to understand. I would probably take the calculation out to reduce the size of the source code line to enhance readability but not really to comment on what was being calculated. If in doubt you could explain the calculation being used.

[MrSwiss]

With regards to taking the vertical and horizontal calculations out of the paramater list ...

This exactly is, where your method "eats speed" (with unnecessary calculations), especially the
vertical calculation. Which only needs to be updated, when the outer loop is run. (simple logic thinking)
The horizontal one, might be debatable, its more, to make the code better readable/commentable.

[SARG]

Code: Select all

screenres 640,480,32
Circle (100,100),5
Circle (140,140),5

pset (100,100)   'set draw coordinates at 100,100
line -(40,40)    'draw relative to 140,140
sleep

Sorry not to be clear but that code draws a line from (100, 100) to (40,40) not (100,100) to (140,140).

And that one does the job using step

Code: Select all

screenres 640,480,32
Circle (100,100),5
Circle (140,140),5

pset (100,100)   'set draw coordinates at 100,100
line step -(40,40)    'draw relative to 140,140
sleep

Extract from user's manual :
"If a pair of coordinates is preceded by the STEP keyword, the coordinates are assumed to be relative to the last graphics cursor position.
"When Line is used as Line - (x2, y2), a line is drawn from the current cursor position to the (x2,y2) coordinates specified by Line. "

[paul doe]

Readability is programmer dependent.

Generally, yes. But you'd have to admit that the checker example is not one of your best codes =D

When I make the effort to do a proper job I think you don't have to be an advanced programmer to read most of my code.

That also depends heavily on the algorithm you use. Take this simple example:

Code: Select all

screenRes( 800, 600, 32 )

const as ulong color_black = rgb( 32, 32, 32 )
const as ulong color_white = rgb( 255, 255, 255 )

sub drawBoard( byval cellSize as integer )
   dim as integer whichCell '' is the cell white or black?
   dim as ulong cellColor '' the cell color
   
   dim as integer cellPosX, cellPosY
   
   '' draw rows
   for y as integer = 0 to 7
      '' determine if the row starts with a black or white cell
      if( y mod 2 ) = 0 then
         whichCell = 1 '' white cell
      else
         whichCell = 0 '' black cell
      end if
      
      '' draw columns
      for x as integer = 0 to 7      
         '' select cell color
         select case as const( whichCell )
            case( 0 )
               cellColor = color_black
            case( 1 )
               cellColor = color_white
         end select
         
         '' draw the cell
         cellPosX = x * cellSize
         cellPosY = y * cellSize
         
         line( cellPosX, cellPosY ) - ( cellPosX + cellSize - 1, cellPosY + cellSize - 1 ), cellColor, bf
         
         '' alternate cell color
         whichCell xor= 1
      next
   next
end sub

drawBoard( 50 )

sleep()


I'm pretty sure you can figure out how to draw the pieces also. It doesn't (and shouldn't) get any more complicated than this.

[BasicCoder2]

Sorry not to be clear but that code draws a line from (100, 100) to (40,40) not (100,100) to (140,140).

Yes sorry I didn't know what I was writing about. I never use the relative LINE stuff and made a mistake.

With regards to taking the vertical and horizontal calculations out of the paramater list ...

This exactly is, where your method "eats speed" (with unnecessary calculations), especially the
vertical calculation. Which only needs to be updated, when the outer loop is run. (simple logic thinking)
The horizontal one, might be debatable, its more, to make the code better readable/commentable.

Yes I agree you need to make sure you are not repeating the same calculation in a loop. Point taken. It is something I tend to look for after I get the code working. I made this mistake with repeating the same trig calculation in the 3D sphere examples.

But you'd have to admit that the checker example is not one of your best codes =D

Maybe. It was the best I could do at the time. I am still happy with it. Although your version uses xor and mod and is a nice example of how they might be used such things do not make it more readable.

I'm pretty sure you can figure out how to draw the pieces also. It doesn't (and shouldn't) get any more complicated than this.

With a modern computer's memory I suspect most would not use code to generate a checker board they would simply display an image.

[dodicat]

My effort.

Code: Select all

Type Pt
    As Single x,y
End Type

Type box
    As Pt p1,p3
    As Ulong c
End Type

Function setgrid(sx As Single,bx As Single,sy As Single,by As Single,st As Single,c1 as ulong,c2 as ulong,p() As box) As long
    #define U Ubound(p)
    Redim p(0)
    dim as single sty=(by-sy)/st,stx=(bx-sx)/st
    bx=sx+st*stx:by=sy+st*sty
    For y As Single=sy To by-1 Step sty
      if st mod 2=0 then swap c1,c2
        For x As Single=sx To bx-1 step stx
            swap c1,c2
            Redim Preserve p(1 To U+1)
            p(u).p1=Type<Pt>(x,y)
            p(u).p3=Type<Pt>(x+stx,y+sty)
            p(u).c=c1
        Next
    Next
    Return 0
End Function

Sub drawgrid(startX as long, _
             startY as long, _
             endX as long, _
             endY as long,_
             size as ulong,_
             colour1 as ulong, _
             colour2 as ulong)
             
             redim as box b()
      setgrid(startX,EndX,Starty,EndY,size,colour1,colour2,b())     
    For n As Long=Lbound(b) To Ubound(b)
        line(b(n).p1.x,b(n).p1.y)-(b(n).p3.x,b(n).p3.y),b(n).c,bf
    Next
End Sub

screen 20,32
drawgrid(10,10,400,400,7,rgb(200,0,0),rgb(0,200,0))
drawgrid(420,420,650,650,3,rgb(200,200,200),rgb(50,50,50))
drawgrid(420,100,720,300,4,rgb(200,200,200),rgb(0,100,255))

sleep

[BasicCoder2]

@dodicat,
Your example is the opposite to what I was alluding to and that was readability to the widest possible number of programmers rather than only readable to programmers skilled in the use of the more abstract and/or advanced programming statements.

Also I would say your example is more of a two coloured checker pattern than say a generic grid. So I would change drawgrid() to drawCheckerPattern()

A generic grid of images may be used to display a checker pattern but not limited to such a function. It could have images of checker, chess or Go pieces or as a display in a game such as the memory game. It might include functions to insert and delete images.

Although a checker board is game related the topic of readability is probably a community discussion and your checker pattern generator is more for Tips and Tricks? Often some good code is lost in the wrong thread particularly if a key search word cannot find it.

Although writing readable code is something close to my heart I will resist writing my thoughts and examples on the topic as a result of the horrendous negative reaction I received on other topics posted to the Community Discussion.

[dodicat]

Not doing too bad yourself Basiccoder2 in the horrendous negative reaction stakes.
Maybe not horrendous, but you are off to a good start.
<joking of course>

@dodicat,
Your example is the opposite to what I was alluding to ...

[BasicCoder2]

@dodicat,
My post was not negative about your code example, on the contrary, it is so good and educational, like all your code, that it shouldn't be lost here thus the suggestion it belongs in Tips and Tricks.

[UEZ]

Different approach.

Short version:

Code: Select all

#Define Floor(x) (((x) * 2.0 - 0.5) Shr 1)

Sub Checkerboard(iSizeX as uShort, iSizeY as uShort, iAmount  as UShort, pScreen As Any Ptr, iColor1 as ULong = &h303030, iColor2 as ULong = &h909090)
   #Define SetPixelCb(_x, _y, _color)  *cptr(ulong ptr, pScreen + _y * pitch_cb + _x Shl 2) = _color
   Dim As Integer w, h, pitch_cb: Dim As Any Pointer imgData_cb: Screeninfo(w, h, , , pitch_cb)
   For y as UShort = 0 to iSizeY * iAmount - 1
      For x as UShort = 0 to iSizeX * iAmount - 1
         SetPixelCb(x, y, Iif(Floor(y / iSizeY) mod 2, Iif(Floor(x / iSizeX) Mod 2 = 0, iColor1, iColor2), Iif(Floor(x / iSizeX) Mod 2 = 0, iColor2, iColor1)))
      Next
   Next
End Sub

screenres 640, 640, 32
Dim pScreen As Any Ptr = ScreenPtr()
Screenlock()
Checkerboard(64, 64, 10, pScreen)
Screenunlock()

Sleep

Readable version:

Code: Select all

'idea by https://www.openprocessing.org/sketch/50928#
#Define Floor(x) (((x) * 2.0 - 0.5) Shr 1)

Sub Checkerboard(iSizeX as uShort, iSizeY as uShort, iAmount  as UShort, pScreen As Any Ptr, iColor1 as ULong = &h303030, iColor2 as ULong = &h909090)
   #Define SetPixelCb(_x, _y, _color)  *cptr(ulong ptr, pScreen + _y * pitch_cb + _x Shl 2) = _color
   Dim As Integer w, h, pitch_cb
   Dim As Any Pointer imgData_cb
   Screeninfo(w, h, , , pitch_cb)
   Dim as Ulong c
   For y as UShort = 0 to iSizeY * iAmount - 1
      For x as UShort = 0 to iSizeX * iAmount - 1
         If Floor(y / iSizeY) mod 2 = 0 Then
            If Floor(x / iSizeX) Mod 2 = 0 Then
               c = iColor1
            Else
               c = iColor2
            End If
         Else
            If Floor(x / iSizeX) Mod 2 = 0 Then
               c = iColor2
            Else
               c = iColor1
            End If
         End If
         SetPixelCb(x, y, c)
      Next
   Next
End Sub

screenres 640, 640, 32
Dim pScreen As Any Ptr = ScreenPtr()
Screenlock()
Checkerboard(64, 64, 10, pScreen)
Screenunlock()

Sleep

[dodicat]

Remember UEZ, for direct pixels, they must be drawn on a locked screen:

screenres 640, 640, 32
Dim pScreen As Any Ptr = ScreenPtr()
screenlock
Checkerboard(64, 64, 10, pScreen)
screenunlock

[UEZ]

Remember UEZ, for direct pixels, they must be drawn on a locked screen:

screenres 640, 640, 32
Dim pScreen As Any Ptr = ScreenPtr()
screenlock
Checkerboard(64, 64, 10, pScreen)
screenunlock

Thanks for the hint -> code updated.