Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add examples to document features #305

Closed
wants to merge 5 commits into from

Conversation

IR0NSIGHT
Copy link
Collaborator

No description provided.

@b-studios b-studios changed the title Documentation Add examples to document features Nov 11, 2023
@b-studios
Copy link
Collaborator

Thank you for your contributions! Here are a few remarks before we can merge this:

  1. we typically do not have single files in the examples folder (which in all honesty is our tests folder). Maybe the features folder might be suitable? What features do you want to demonstrate? How would you call the files
  2. to automatically run the files and check whether our compiler still works, all files have to have a .check file with the expected println output. If you don't print anything, the file can be empty.
  3. we don't have a styleguide, but indent all Effekt files with 2 spaces. I'll also add a few more stylistic comments in the review.

import immutable/list

interface DataBase[R] {
//will save name to database
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//will save name to database
// will save name to database

why "name"?

Student(firstName, lastName, nextId());
}

db.addEntry(newStudent("Max","Marschall"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
db.addEntry(newStudent("Max","Marschall"));
db.addEntry(newStudent("Max", "Marschall"));

db.addEntry(newStudent("Pierre","Drole"));
db.addEntry(newStudent("Airan","Sayüt"));

db.printEntries[Student]()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the type annotation necessary?

println(db.getEntries[Student]())
}

def makeNewDB(){ r: Region }:DataBase[Student] at { r } = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def makeNewDB(){ r: Region }:DataBase[Student] at { r } = {
def makeNewDB { r: Region }: DataBase[Student] at { r } = {

}

def makeNewDB(){ r: Region }:DataBase[Student] at { r } = {
//create database instance
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//create database instance
// create database instance

def firebase = new DataBase[Student] {

def addEntry(entry: Student) = {
names = Cons[Student](entry, names)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, is the type annotation necessary?

}

def printEntries() = {
names.foreach(){ name => println(name); ()}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
names.foreach(){ name => println(name); ()}
names.foreach { name => println(name) }

Comment on lines +74 to +76
def getEntries() = {
return names;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def getEntries() = {
return names;
}
def getEntries() = names

};

region r {
def fb2 = makeNewDB(){ r };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def fb2 = makeNewDB(){ r };
def fb2 = makeNewDB {r};

Typically we omit empty value sections

var time = currentTimeNanos();
var timeMillis = time / 1000000;
println("time millis: " ++ show(timeMillis))
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

printing the time in a "test" is difficult because you don't know what the expected result will be

@b-studios
Copy link
Collaborator

I am writing such detailed review comments since it would be great if you could also apply them to Effekt code that you write in general :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants