CodenameOne
CodenameOne copied to clipboard
fantastically inefficient code
I'm developing some new code that makes heavy use of StringBuffer, and specifically StringBuffer.subSequence to implement the StringBuffer.subString method (which the Codenamame1 api doesn't have).
When deployed on real hardware seems to garbage collect itself to death. I found this, which seems to be the implementation actually used
public CharSequence subSequence(int start, int end) {
return toString().substring(start, end);
}
How about this instead?
public static String substring(StringBuilder str,int from,int to)
{ int len = to-from;
char seq[] = new char[len];
for(int i=0;i<len;i++) { seq[i] = str.charAt(from++); }
return(new String(seq));
}
ps, also applicable to StringBuffer
You can submit a PR with that change if you want.
Having identified the problem, it's no longer a problem or a priority for me.
Can I work on this?
Sure
Could you help point me to the file?
This: https://github.com/codenameone/CodenameOne/blob/master/vm/JavaAPI/src/java/lang/StringBuffer.java But also this: https://github.com/codenameone/CodenameOne/blob/master/vm/JavaAPI/src/java/lang/StringBuilder.java
Thank you
Hi @shai-almog , I forked and cloned the repo, added the requested method to both classes. Cool to submit a PR?
I hope you noticed that the constructors probably need work too.
public StringBuffer StringBuffer(java.lang.CharSequence cs) { return new StringBuffer(cs.toString()); }
@onesirian you can always submit a PR. If it has a problem we'll comment on the PR on review and you can fix it.
hi @ddyer0
I am beginner to opensource and I would like to contribute to issue if it is still open.
@Ejaz29 I understand @onesirian already did this but we still didn't get the PR from him.
@shai-almog Hi, has this been fixed yet, or can I take a crack at it?
No. It seems there was no PR on this.
No. It seems there was no PR on this.
Hey if anyone is not working on this issue I would like to try it. /assign
Collaborator
can I try this issue?
A PR still hasn't been submitted...
In StringBuffer::subSequence why not use: internal.substring(start, end)
In the StringBuilder constructors there doesn't seem to be a reason to support String directly since a String is a CharSequence. Why not just use the constructor that takes a CharSequence with the following: StringBuilder(CharSequence cs) { count = cs.length; value = new char[count ]; for(int i=0;i<count ;i++) { value [i] = cs.charAt(i++); } }
In StringBuilder::subsequence why not use the following to create one string instance instead of two: public CharSequence subSequence(int start, int end) { int length = end - start; return new String(value, start, length); }
Hi I have not seen this issue solved in the current version (8.0 - Snapshot). I have submitted a PR with some basic changes trying to address this issue. @ddyer0 I know that it is now more than a year and a half after you started this issue, but if you're still trying to create an implementation for substring in StringBuffer I would love to help out. I have also noticed that there are other methods that are in StringBuffer and StringBuilder in Oracle's JavaDocs that are not currently implemented. Thank you, looking forward to receive feedback/updates 😄
My practice for things like this is to report them and solve them for my own use. I generally only go the the hassle of making push requests and negotiating with codename1 for things that are not so easy to fix separately. So yea, there are lots of things in codename1's base that could be closer to standard java, but they're not anyone's priority. Its a good learning experience to go through the hassle of helping out a few times :)
Thank you @ddyer0. I will try to make some useful implementation since I have the time 😝. I'll also keep in mind that it may be better to create simple solutions, so that it is always easy to modify/implement the code for one's personal use.
Hey, can I work on this issue? I am new to open source can you guide me more regarding this issue.
I'm not sure what @chloeimb is doing here. I assumed he closed the PR with the intention of fixing/resubmitting... It's actually pretty close to what is needed here.
For anyone going forward with this issue:
@chloeimb must have noticed my solution may not propagate to all ports if any of the ports redefine the classes pertinent to this issue. ( Check Ports/CLDC11/src/java/lang/String.java:427-9 for example. ) However, I'm not sure if this is the case. I have not worked with java and ports before.
I would recommend checking out my commit earlier on this thread for ideas on how to do this.
Only the code in JavaSE API matters since the other ports have efficient code for that...
@shai-almog hey, i actually started working on this for a school project that required a open source contribution, but closed it pretty quick bc i knew the code wasn't great and didn't want to clog your pull requests with it. i'd be more than happy to resubmit and make changes though i actually planned on coming back to look at this more seriously and get it working
@OctavioAB I am not sure what the fix you made was, i had made some local port changes since i was having build issues on my unix machine, but that was not the main change that i was making.
Hey, can I work on this issue ? Is it still open ??
I am beginner to opensource and I would like to contribute to issue if it is still open.
@emanuelconte assuming @chloeimb has given up you can try.
can i contribute to this issue?