Conversation
|
Hi, would you like to add a timezone variable to |
| Value v = ss.pop(); | ||
| Object o = null; | ||
| switch (v.type) { | ||
| case Null: o = Value.null_(); break; |
There was a problem hiding this comment.
this is wrong, should just be o = null
There was a problem hiding this comment.
this is wrong, should just be
o = null
iirc I used this because I saw Value.null_() being used somewhere else. Anyways do you want me to edit and fix this, if you are going to accept this pr?
There was a problem hiding this comment.
For me personally it would be really nice, if this PR is being merged. :)
There was a problem hiding this comment.
this is wrong, should just be
o = nulliirc I used this because I saw
Value.null_()being used somewhere else. Anyways do you want me to edit and fix this, if you are going to accept this pr?
Value.null_() is the starscript null type, but in this case you’re feeding it into the java String.format() method, which takes in the java null
There was a problem hiding this comment.
Check my new commits in case you're interested in accepting this PR. Should be fixed now.
Java |
|
Yeah, I mean the option to switch the timezone. |
I added that as an optional argument to |
|
|
||
| public static Value format(Starscript ss, int argCount) { | ||
| if (argCount < 1) ss.error("format(fmt, ...args) requires at least 1 argument, got %d.", argCount); | ||
| ArrayList<Object> args = new ArrayList<Object>(); |
There was a problem hiding this comment.
the current implementation is less than ideal in performance
- using the no-arg
ArrayListconstructor means this method will potentially cause several object arrays to be allocated as the list will have to be resized - inserting objects at index 0 means that the other elements will have to be offset constantly
it would be better to just use an object array directly and filling it with a reverse for i loop
| case String: o = v.getString(); break; | ||
| case Function: o = v.getFunction(); break; | ||
| case Map: o = v.getMap(); break; | ||
| case Null: |
There was a problem hiding this comment.
instead of initializing Object o as null and simply breaking out of the switch on case Null/default, it would be more readable if o wasn't initialized to a value, but case Null/default set it to null
|
Fixed. Any more suggestions? |
3 participants
This commit adds
formatandformatDateTimeto Starscript. Yes, I know one of the selling points of Starscript is to be faster thanString.format, but sometimes you do need more formatting options.format(fmt, ...args)
Adds arbitrary formatting by calling
String.format. This can be considered as a fix to [#7] too.formatDateTime(fmt)
Calls
SimpleDateFormatbecause currently Starscriptdateandtimedon't allow the user to choose a custom format.