Ticket #273 (closed enhancement: fixed)

Opened 8 years ago

Last modified 7 years ago

Code commenting and New Year tweaking

Reported by: hopscc Owned by: Chuck
Priority: medium Milestone:
Component: Cobra Compiler Version: 0.8.0
Keywords: Cc:


Add docstrings to RTL code (Extensions files) so some info will show in doc gen of these (StandardLibraryExtensionMethods).

Fix typo in Nodes.cobra comment
Add docstrings/comments to props and methods in CobraParser?.cobra

Add stack-like methods (push/pop) as convenience extensions on List
Additional Test for this.

Implement some TODO code - checks and error messages for multiple protection access modifiers, incompatible explicitly specified accessor modifiers
and trap and emit message for invalid/unsupported name form for short form properties explicitly naming property backing variables.
New tests for these.


tweaks.patch Download (19.7 KB) - added by hopscc 8 years ago.

Change History

Changed 8 years ago by hopscc

Changed 8 years ago by hopscc

  • status changed from new to assigned
  • owner set to Chuck

Changed 8 years ago by Charles

Looking at this now.

Changed 8 years ago by Charles

  • owner changed from Chuck to hopscc

So regarding the new error on:

class A
	var mlog
	pro getLog from mlog 
	# .error. names must begin with an underscore prefix	

Are you
(a) Objecting to making a "pro FOO from BAR" where "BAR" is a public variable? OR,
(b) Just preventing the C# code gen error?

Changed 8 years ago by hopscc

  • owner changed from hopscc to Chuck

Neither primarily. From memory the original impetus was an observation that the code around this didnt support correctly a name without a leading underscore.
The fact that it (perhaps consequently) didnt codegen correctly (b) and didnt make much sense(a) and hadnt surfaced before seemed to indicate that it would be desirable to check and suppress explicitly.

Changed 8 years ago by hopscc

  • summary changed from Code commenting and Near Year tweaking to Code commenting and New Year tweaking

fix ticket title typo.

Changed 8 years ago by Charles

  • status changed from assigned to closed
  • resolution set to fixed

This is complete via a couple checkins.

I left out the .push and .pop List extension methods for a few reasons:
(1) There is already a Stack data structure with .push and .pop as well as .peek.
(2) The abscence of .peek in the List extensions just highlights on how making Lists be Stack-like could go on indefinitely.
(3) Having .push be a straight cover for .add feels wrong.

Changed 8 years ago by hopscc

  • status changed from closed to reopened
  • resolution fixed deleted

I provided them for a few different reasons.
The stack named extension methods ( and possibly queue named ones ) are a convenience and to allow advertisement of intent.
( i.e This data structure is a list but its being used (only) as a stack,
eventually if necessary it may change to the real data structure (stack/queue) and gain any optimizations from that)

You can always (and people do) treat lists as stacks or queues using the existing API, the (possibly duplicated function) stack like (or queue like) names are to allow someone to explicitly specify what you are using them as and to easily allow exploration/experimentation/migration of data structure (albeit perhaps non optimised) without having to change BOTH declarations AND method calls to do so.
(List -> Stack, List ->Queue)

I thought it a toss up whether peek or list[list.count]) or list.last was any clearer for a top of stack peek function vs (list[n]) flexibility of allowing peek to any part of a stack
thats possibly too much so just add a peek method duplicating list[count]
- what other (common) stack ops are there that might make this go on indefinitely??

I'd suggest .push doing same as .add only feels wrong because it looks like a duplication of the API function ( which it is).
If you think of it instead as being an explicit choice to describe intention to use as a stack its entirely natural.

l.add(value) # push onto top of stack

(reopened (perhaps temporarily) to hilite this comment added)

Changed 8 years ago by Charles

Okay I agree with you that this wouldn't go on indefinitely since there are only so many stack operations.

Python covers stacks and queues in Sections 5.1.1 and 5.1.2 at They have a pop but don't bother adding covers for their append or t[-1]. For queues, they correctly point out that lists are very inefficient for this, so they point you to their queue class.

Since our List already has add and last, I think we only need pop. If people want expressive names, they can use the "correct" class or add extension methods that are relevant to their purposes.

Changed 8 years ago by hopscc

Fair enough.

add == push, last == peek.
I'd like to hold out for providing 'push' just for clarity for use as a stack
but I guess I can live with only 'pop' for the release distribution.

I'll add a task for myself to augment the cobra wiki with a datastructures topic that
covers the above discussion.

Changed 7 years ago by Charles

  • status changed from reopened to closed
  • resolution set to fixed

I have added List.pop.

Changed 7 years ago by Charles

Changed 7 years ago by Charles

Note: See TracTickets for help on using tickets.