CodenameOne icon indicating copy to clipboard operation
CodenameOne copied to clipboard

fantastically inefficient code

Open ddyer0 opened this issue 4 years ago • 32 comments

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

ddyer0 avatar Feb 17 '21 03:02 ddyer0

You can submit a PR with that change if you want.

shai-almog avatar Feb 17 '21 18:02 shai-almog

Having identified the problem, it's no longer a problem or a priority for me.

ddyer0 avatar Feb 17 '21 18:02 ddyer0

Can I work on this?

pavanp73 avatar Sep 11 '21 05:09 pavanp73

Sure

shai-almog avatar Sep 11 '21 05:09 shai-almog

Could you help point me to the file?

pavanp73 avatar Sep 11 '21 06:09 pavanp73

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

shai-almog avatar Sep 11 '21 06:09 shai-almog

Thank you

pavanp73 avatar Sep 11 '21 13:09 pavanp73

Hi @shai-almog , I forked and cloned the repo, added the requested method to both classes. Cool to submit a PR?

lawrencedcodes avatar Oct 01 '21 22:10 lawrencedcodes

I hope you noticed that the constructors probably need work too.

public StringBuffer StringBuffer(java.lang.CharSequence cs) { return new StringBuffer(cs.toString()); }

ddyer0 avatar Oct 01 '21 22:10 ddyer0

@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.

shai-almog avatar Oct 02 '21 04:10 shai-almog

hi @ddyer0
I am beginner to opensource and I would like to contribute to issue if it is still open.

Ejaz29 avatar Oct 17 '21 13:10 Ejaz29

@Ejaz29 I understand @onesirian already did this but we still didn't get the PR from him.

shai-almog avatar Oct 17 '21 18:10 shai-almog

@shai-almog Hi, has this been fixed yet, or can I take a crack at it?

aneeshp4 avatar May 19 '22 04:05 aneeshp4

No. It seems there was no PR on this.

shai-almog avatar May 20 '22 03:05 shai-almog

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

AakashRaj20 avatar Jun 10 '22 10:06 AakashRaj20

Collaborator

can I try this issue?

AakashRaj20 avatar Jun 10 '22 14:06 AakashRaj20

A PR still hasn't been submitted...

shai-almog avatar Jun 10 '22 18:06 shai-almog

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); }

itsreallylit avatar Sep 05 '22 04:09 itsreallylit

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 😄

OctavioAnino avatar Oct 01 '22 21:10 OctavioAnino

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 :)

ddyer0 avatar Oct 01 '22 21:10 ddyer0

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.

OctavioAnino avatar Oct 02 '22 02:10 OctavioAnino

Hey, can I work on this issue? I am new to open source can you guide me more regarding this issue.

Rishabh-mikku avatar Oct 04 '23 15:10 Rishabh-mikku

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.

shai-almog avatar Oct 05 '23 02:10 shai-almog

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.

OctavioAB avatar Oct 06 '23 01:10 OctavioAB

Only the code in JavaSE API matters since the other ports have efficient code for that...

shai-almog avatar Oct 06 '23 02:10 shai-almog

@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

chloeimb avatar Oct 06 '23 06:10 chloeimb

@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.

chloeimb avatar Oct 06 '23 06:10 chloeimb

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 avatar Jan 26 '24 09:01 emanuelconte

@emanuelconte assuming @chloeimb has given up you can try.

shai-almog avatar Jan 27 '24 05:01 shai-almog

can i contribute to this issue?

Ahmadmidlaj avatar Feb 09 '24 14:02 Ahmadmidlaj