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.

Sign up for GitHub

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

Optimize array_contains functions #1395

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Optimize array_contains functions #1395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/com/laytonsmith/core/constructs/CArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ public Version since() {
@Override
public Iterator<Mixed> iterator() {
if(associativeMode) {
throw new RuntimeException("iterator() cannot be called on an associative array");
return associativeArray.values().iterator();
Copy link
Member

Choose a reason for hiding this comment

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

This change worries me. Inherently, it's not possible to generally say whether someone intended to iterate the keys or values when getting the iterator, and I think it's reasonable to assume the opposite of whichever way we choose here. Indeed, Java's Map doesn't directly implement Iterable for a reason (though it provides entrySet, values, etc, to make it easy to iterate in various ways). I know methodscript arrays are terribly overloaded, but I think trying to directly iterate on an associative array is a sign that you're doing something you shouldn't, and so this should remain an error. array_keys and array_normalize exist, allowing people to iterate in either way, albeit not particularly lazily. Eventually though, there will be a standard object library, which will include proper (and non-magical) maps, sets, etc, with proper iterators, but for now, I would prefer to keep this strict and not allow direct iteration on an associative array.

} else {
return array.iterator();
}
Expand Down
33 changes: 32 additions & 1 deletion src/main/java/com/laytonsmith/core/constructs/CByteArray.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,20 @@ public void removeValues(Mixed construct) {

@Override
public Iterator<Mixed> iterator() {
throw new CREUnsupportedOperationException("Iterating a byte array is not supported.", getTarget());
return new Iterator<Mixed>() {

private int index = 0;

@Override
public boolean hasNext() {
return this.index <= maxValue;
}

@Override
public Mixed next() {
return new CInt(data.get(this.index++), Target.UNKNOWN);
}
};
}

@Override
Expand Down Expand Up @@ -696,6 +709,24 @@ protected SortedMap<String, Mixed> getAssociativeArray() {
throw new Error("This error should not happen. Please report this bug to the developers");
}

@Override
public Iterator<Mixed> iterator() {
return new Iterator<Mixed>() {

private int index = 0;

@Override
public boolean hasNext() {
return this.index <= backing.length;
}

@Override
public Mixed next() {
return new CInt(backing[this.index++], Target.UNKNOWN);
}
};
}

@Override
public String docs() {
return "A read-only subclass of array, which is used to make reading byte arrays more efficient.";
Expand Down
37 changes: 37 additions & 0 deletions src/main/java/com/laytonsmith/core/constructs/CSlice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,43 @@ public int size() {
};
}

@Override
public Iterator<Mixed> iterator() {
if(start <= finish) {
return new Iterator<Mixed>() {

private long current = start;
private long last = finish;

@Override
public boolean hasNext() {
return this.current <= this.last;
}

@Override
public Mixed next() {
return new CInt(this.current++, Target.UNKNOWN);
}
};
} else {
return new Iterator<Mixed>() {

private long current = start;
private long last = finish;

@Override
public boolean hasNext() {
return this.current >= this.last;
}

@Override
public Mixed next() {
return new CInt(this.current--, Target.UNKNOWN);
}
};
}
}

@Override
public long size() {
return size;
Expand Down
42 changes: 33 additions & 9 deletions src/main/java/com/laytonsmith/core/functions/ArrayHandling.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,9 +712,17 @@ public Mixed exec(Target t, Environment env, Mixed... args) throws CancelCommand
CArray ca = ArgumentValidation.getArray(args[0], t);
aa = ca;
}
for(Mixed key : aa.keySet()) {
if(new equals().exec(t, env, aa.get(key, t), args[1]).getBoolean()) {
return CBoolean.TRUE;
if(aa instanceof CArray ca) {
for(Mixed val : ca) {
if(new equals().exec(t, env, val, args[1]).getBoolean()) {
return CBoolean.TRUE;
}
}
} else {
for(Mixed key : aa.keySet()) {
if(new equals().exec(t, env, aa.get(key, t), args[1]).getBoolean()) {
return CBoolean.TRUE;
}
}
}
return CBoolean.FALSE;
Expand Down Expand Up @@ -822,9 +830,17 @@ public Mixed exec(Target t, Environment environment, Mixed... args) throws Confi
} else {
aa = ArgumentValidation.getArray(args[0], t);
}
for(Mixed key : aa.keySet()) {
if(new equals_ic().exec(t, environment, aa.get(key, t), args[1]).getBoolean()) {
return CBoolean.TRUE;
if(aa instanceof CArray ca) {
for(Mixed val : ca) {
if(new equals_ic().exec(t, environment, val, args[1]).getBoolean()) {
return CBoolean.TRUE;
}
}
} else {
for(Mixed key : aa.keySet()) {
if(new equals_ic().exec(t, environment, aa.get(key, t), args[1]).getBoolean()) {
return CBoolean.TRUE;
}
}
}
return CBoolean.FALSE;
Expand Down Expand Up @@ -876,9 +892,17 @@ public Mixed exec(Target t, Environment env, Mixed... args) throws CancelCommand
} else {
aa = ArgumentValidation.getArray(args[0], t);
}
for(Mixed key : aa.keySet()) {
if(new sequals().exec(t, env, aa.get(key, t), args[1]).getBoolean()) {
return CBoolean.TRUE;
if(aa instanceof CArray ca) {
for(Mixed val : ca) {
if(new sequals().exec(t, env, val, args[1]).getBoolean()) {
return CBoolean.TRUE;
}
}
} else {
for(Mixed key : aa.keySet()) {
if(new sequals().exec(t, env, aa.get(key, t), args[1]).getBoolean()) {
return CBoolean.TRUE;
}
}
}
return CBoolean.FALSE;
Expand Down
Loading