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

Added a Date Class for date related methods #95

Closed
wants to merge 2 commits into from

Conversation

hallzy
Copy link
Contributor

@hallzy hallzy commented Mar 17, 2017

I thought that having a Date class would be useful. Basically gives you information like the current epoch time, current hour, minute, and second of the day, day of the month, the current month etc.

I also added a doc for it as a new file. I put it under the "System Class" in the left sidebar since it is a built in class and I figured they should be close, but it is also under advanced, and Date may not really apply as advanced.

Also, I think it would be interesting to have a a few properties in this class such as something along the lines of "human_readable" which would give the actual full name like "January" "February" "Sunday" etc instead of the numbers associated with them. I do not have this in this PR though.

Also, I did not forget to write tests for this... I just am not sure how I would go about testing this because the numbers are based on the time and date right now, so the values always change. The only idea I have come up with as a test is just to write a test that checks that the value that is returned is a valid value for that method... for example, have a test that fails if Date,month() gets to 13 since, obviously there isn't 13 months.

@hallzy
Copy link
Contributor Author

hallzy commented Mar 17, 2017

I have actually now added the human_readable property... I have pushed it to a different branch on my fork here hallzy@885688d because I am not exactly sure that it was done as you would prefer,.

The easiest way for me to get the property to work was to do it in a similar way to how the garbage collection stuff was done for System, so I ended up having the human_readable inside the gravity_vm struct.

On top of that, I needed to check the state of it from my functions in gravity_core.c so I had to move the entire struct from the gravity_vm.c file to the gravity_vm.h file by doing it this way (for example, to get the month, in the function that returns the month, i did if (vm->human_readable == true) to check whether I should return the integer or string.

tests all still pass with this change though.

@marcobambini
Copy link
Owner

Date class is really helpful and we could also override standard operators to add nice features like date1 + date2. However the gravity_core.c file should be used only for core classes required to run a standard VM.

We should introduce a new folder optional or lib in order to have a central repository for all the new classes, we could also add random and file classes.

@marcobambini
Copy link
Owner

For the human_readable flag, instead of modify the VM you could check for an optional bool parameter (false by default). For example for the date_month example:

static bool date_month (gravity_vm *vm, gravity_value_t *args, uint16_t nargs, uint32_t rindex) {
   bool human_readable = false;
   if (nargs == 2) and (VALUE_ISA_BOOL(GET_VALUE(1))) human_readable = VALUE_AS_BOOL(GET_VALUE(1));
  ...
}

and user can write the following Gravity code:

// no human readable
t = Date.month()

// human readable
t = Date.month(true)

@marcobambini
Copy link
Owner

Added the optional folder inside src folder

@hallzy
Copy link
Contributor Author

hallzy commented Mar 17, 2017

Good to know, I will probably not be able to get to this until Saturday however.

Also your idea about the Boolean as an argument is something I thought about, and it was between the human_readable and that. I can definately change it to that though.

Would you say that the default behavior (not passing any argument) should be the readable form?

@marcobambini
Copy link
Owner

Mine was just an example, I am not sure about what should be the correct behavior.

@hallzy
Copy link
Contributor Author

hallzy commented Mar 17, 2017

Also, your example for a date1+date2 would be useful, but given the current implementation I don't think that is possible. What I have written so far I do not believe would support a date object. Right now they are really just methods to get the current date information.

For sure though, being able to make Date objects would be useful

#pragma unused(args,nargs)
time_t t = time(NULL);
// Plus 1 because tm_wday is a base 0 number
int tm = localtime(&t)->tm_isdst;
Copy link

@InusualZ InusualZ Mar 17, 2017

Choose a reason for hiding this comment

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

You forgot to remove the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved. thanks

static bool date_year_day (gravity_vm *vm, gravity_value_t *args, uint16_t nargs, uint32_t rindex) {
#pragma unused(args,nargs)
time_t t = time(NULL);
// Plus 1 because tm_wday is a base 0 number

Choose a reason for hiding this comment

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

You forgot to rename the variable in the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved, thanks

@hallzy
Copy link
Contributor Author

hallzy commented Mar 17, 2017

I actually probably won't be able to get to this for a while to make the necessary changes because of some university projects. If someone wants to take over to make the necessary changes, that would be quite alright.

Also not really sure how you want to go about connecting these separate files to the rest if we are separating from the core stuff. At the moment I don't think I know enough of the code well enough to connect it.

Also, adding the ability for a constructor might be useful... so for example:

var d1 = Date() // creates an object with the date information of right now

var d2 = Date(1489795621) // creates an object with the date information that corresponds to the specific epoch time

var d3 = Date(2017, 3, 17, 17, 8, 40) // creates a date object for 5:08:40 on March 17th 2017... epoch time, and daylight savings status, and week day etc would be calculated based on the input information

// etc

Just some ways to create objects with different kinds of information

@marcobambini
Copy link
Owner

@hallzy I collected all your code in my local repository, I'd like to find out a way to automatically get all optional code included into the main gravity executable. I am going to close this pull request without merging it but I'll add your date class as soon as I can (see my comments in https://github.com/marcobambini/gravity/wiki/Roadmap)

Thanks a lot for your contribution!

@marcobambini
Copy link
Owner

Just added a way to use optional classes 72c66a2
Take a look at the Math class for a template to use.

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.

3 participants