[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 168
  • Last Modified:

Sort out my code

Hi there,

I understand that im probably going to get no responses but here goes nothing!

I have written some very bad, inefficient code that does exactly what i need it to but i need to neaten it up alot.
It is basically a program which talks to a measureing device over bluetooth, gets a result and sends it to a web server.
At the moment it is one giant file with parts of the code repeated up to 4 times.  I know it could be much shorter and efficient but I have tried to split it up into seperate java files several times with disaterous consequences. When it comes to java - i am not the best.  

I understand people probably wont want to trawl through it all but if i could get started then it would be ... well - a start.

Have placed the code at the end of this link as it would be too big to put here really (its about 1000 lines)
http://www.geocities.com/mingbaden/J2ME_CODE.html

Big thanks to anyone who even attempts this.

Cheers

Chris
0
mingbaden
Asked:
mingbaden
  • 8
  • 3
1 Solution
 
OBCTCommented:
I'm not going to re-write your code for you because it's against the EE regulations but I'm having a look at it now and I'll help at as much as possible.

First of all, you have alot of calls to Display.getDisplay(this), you could make this quicker by adding a Display member to your class and a small method to set the Displayable object.
For example...

private Display display; // Member declaration
...

/**
  * Sets the MIDlets Displayable object.
  * @param d The Displayable object to be set on the screen.
  */
public void setDisplayable(Displayable d)
{
    display.setCurrent(d);
}

// Change your startApp method to the following so that the Display gets set.
public void startApp()
{
    if (display == null)
    {
        display = Display.getDisplay(this);
    }

    show();
}

I'll make more posts with the other things I see.
0
 
OBCTCommented:
BTW, with that addition, you'll be able to change calls from...
Display.getDisplay(this).setCurrent(myDisplayableObject);
to...
setDisplay(myDisplayableObject);
0
 
OBCTCommented:
>setDisplay(myDisplayableObject);

was meant to be...
setDisplayable(myDisplayableObject);

Sorry, I'm not a morning person. ;-)
0
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
OBCTCommented:
In your commandAction(Command, Displayable) method, the one implemented from CommandListener, you have alot of return statements that aren't really nessicary.
This is whats being done currently...

if (c == command1)
{
    // Do something
    return;
}

if (c == command2)
{
    // Do something else
    return;
}

By making one 'if' statement and the rest 'else if' statements, you'll be saving yourself from needing to call 'return'.
E.g.

if (c == command1)
{
    // Do something
}
else if (c == command2)
{
    // Do something else
}
0
 
OBCTCommented:
In a few places, you have the following code...

Reader in = new InputStreamReader(is);
int in_bin;
String instr = "";
                        
while ((in_bin = in.read()) != 13)
{
    instr += (char) in_bin;
}

In J2ME, this causes problems. instr += (char) in_bin doesn't actually append the character to the original String, but creates a new String because Strings are immutable (can't be changed).
Instead, use a StringBuffer and it will save you alot of overhead.
E.g.

Reader in = new InputStreamReader(is);
int in_bin;
StringBuffer buffer = new StringBuffer();
                        
while ((in_bin = in.read()) != 13)
{
    buffer.append((char) in_bin);
}
0
 
OBCTCommented:
Theres also alot of code being repeated in your commandAction() method.
Two methods really should be created to save yourself rewriting everything.

public void write(String msg, OutputStream os)
{
    Writer out = new OutputStreamWriter(os);
    out.write(msg);
    out.close();
    // Theres no need to flush the writer because closing it will automatically flush it.
}

You would use the write(String, OutputStream) method like so...

write("p\r\n", ((StreamConnection) Connector.open(ServiceURL)).openOutputStream());

The second method to create would be a read method.

public String read(InputStream is)
{
    Reader in = new InputStreamReader(is);
    int in_bin;
    StringBuffer buffer = new StringBuffer();
      
    while ((in_bin = in.read()) != 13)
    {
        buffer.append((char) in_bin);
    }

    in.close();
            
    return buffer.toString().substring(7, buffer.length() - 1);
}

You would call this method like so...

String data = read(((StreamConnection) Connector.open(ServiceURL)).openInputStream());

Apart from all that I've said so far, everything else seems generally ok. Theres nothing obvious that I can see that would cause any major problems.
If there is code that has been re-written, it's a good idea to put it into a method of it's own. Everything will be far easier to read and debug.
I hope this gets you on your way.

Cheers.

-OBCT
0
 
mingbadenAuthor Commented:
Cheers OBCT
That is enough to get me started.
Will let you know how i get on.

Chris
0
 
mingbadenAuthor Commented:
Thats all worked well - cheers for that.

how would i go about putting method like the one above, and some of the form and menu creation statements in their own .java files that are called by the main one.

This would allow me to have smaller, more readable files and allow me to test them seperately.

Can i put calls to other .java files in the main constructor - that is where i really had problems before.

Cheers.

Chris
0
 
OBCTCommented:
If I understand correctly, you can extend Forms and Menus like you would with any other java class.
For example...

public class MyCustomForm extends Form
{
    private MainMIDlet parent; // I personally find it handy to have a reference to your main MIDlet so the form can access any needed functionality.

    public MyCustomForm(String title, MainMIDlet parent)
    {
        super(title);

        this.parent = parent;

        // From this point on, you would add your desired items, set any CommandListeners etc
        append(myFormItem);
        setCommandListener(parent);
    }
}

>Can i put calls to other .java files in the main constructor

You certainly can.
E.g.

/* The MainMIDlet's constructor */
public MainMIDlet()
{
    myCustomForm = new MyCustomForm("Example Form", this);
}

Is this what you mean?
0
 
mingbadenAuthor Commented:
Hi there,

i think that is what i mean but when i try to do the above for a form (in this case - login) it cannot resolve the symbol login (login = new LoginForm("Login Form", this);) in the command listener method- i think this is the problem i had before.  Do i need to declare it at the beginning as public? ...... or private? ...  or something

\src\MainMIDlet.java:227: cannot resolve symbol
symbol  : variable login
location: class MainMIDlet
                             setDisplayable(login);
                                               ^
\src\MainMIDlet.java:239: cannot resolve symbol
symbol  : variable login
location: class MainMIDlet
                if (d==login)
                       ^

Cheers

Chris
0
 
OBCTCommented:
In your class you'll need to declare 'login' as a member. Private would be a suitable for what your doing.
If it's been declared in your class, then there would be no reason for the compiler to spew out errors.
Make sure your code is something like this...

public class MainMIDlet extends MIDlet
{
    private LoginForm login;

    ...

    public MainMIDlet()
    {
        login = new LoginForm("Login", this);
    }

    ...

    public void commandAction(Command c, Displayable d)
    {
        if (c == login)
        {
        }
    }
}

If this is what you already have, then please post your code so I can take a closer look.
0

Featured Post

Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

  • 8
  • 3
Tackle projects and never again get stuck behind a technical roadblock.
Join Now