aspose file tools*
The moose likes Beginning Java and the fly likes Ugly code and stuck Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Ugly code and stuck" Watch "Ugly code and stuck" New topic
Author

Ugly code and stuck

Daniel Flannigan
Greenhorn

Joined: Jan 22, 2013
Posts: 9
*This is an edit, but sorry, I dont know how to put the code into a nice numbered lines format like others have.*

I am finishing up an assignment for my class and I have all of my information set in methods and variables (I think). I have been working on this assignment for 2 weeks now without anyones help. But it is now due very soon and I am turning to the experts to try and point me in the right direction. The assignment is a salesman array posting what they sold that year, their commission rates they earned, and a return table showing up to 50% more of what they could be making had their sales been higher. I have a folder of all of the info. It is only two (main and class). I have to save them as .txt files to submit them here. I hate to ask others for a direction, but before 3 weeks ago I have never programmed at all. Any help in direction or process would be very appreciated. What am I doing wrong here besides chopping up this program code into ugly chunks? here are the guidelines: Thanks again.

The company has recently changed its total annual compensation policy to improve sales.
• A salesperson will continue to earn a fixed salary of $30,000 per year. The current sales target for every salesperson is $120,000 minimum per year.
• The sales incentive will only start when 80% of the sales target is met. The current commission is 12.5% of total yearly sales.
• If a salesperson exceeds the sales target, the commission will increase based on an acceleration factor. The acceleration factor is 1.25.
• The application should ask the user to enter annual sales, and it should display the total annual compensation.
• The application should also display a table of potential total annual compensation that the salesperson could have earned, in $5000 increments above the salesperson’s annual sales, until it reaches 50% above the salesperson’s annual sales.

The application will compare the total annual compensation of at least two salespersons.
• It will calculate the additional amount of sales that each salesperson must achieve to match or exceed the higher of the two earners.
• The application should ask for the name of each salesperson being compared.
• The application should have at least one class, in addition to the application’s controlling class.
• The source code must demonstrate the use of Array or ArrayList.
• There should be proper documentation in the source code.




Aj Prieto
Ranch Hand

Joined: Sep 28, 2012
Posts: 72

Here is how to make your code much easier to read: UseCodeTags (<----Click)

Also, I admit that I haven't looked deeply into your code, but what problems are you having with it?


Da mihi sis bubulae frustum assae, solana tuberosa in modo Gallico fricta ac quassum lactatum coagulatum crassum.
Daniel Flannigan
Greenhorn

Joined: Jan 22, 2013
Posts: 9
I am not using the code efficiently in the main. I think that I could do the IF ELSE statements a lot better and maybe combine them differently if I knew how to do that. Also, I cannot figure out how to get the calculations to work on the end results. As I said, I have all of the information there, but I am not sure how to put it all together using the code that I know. I saw reference to JFrames and statements that I have no idea what they were, nor being able to try such methods. I am just stuck with my output looking horrible and incomplete.
Daniel Flannigan
Greenhorn

Joined: Jan 22, 2013
Posts: 9
This is what I think the code is supposed to look like in the end. I am also missing an array but that seems to difficult for my understanding right now. Learning java on your own is not easy.

Aj Prieto
Ranch Hand

Joined: Sep 28, 2012
Posts: 72

JFrame is for when you start to make GUIs.

As for the if-else statements, you could use "switch" statements, which would look nicer, in my opinion. You also have an extra set of "{}" after the "if (name.equals(name))".

I would take the that whole block of if-else statements and throw that into a method so that your main method would be a lot cleaner.

You should probably look at the cases in your if-else statements. Specifically, what would happen if annualSales were 120,000.00.

Also, you shouldn't have to java on your own. You say that you're taking a class, why not ask the instructor or the other students for help? It might help you to understand things because you could get instant feedback.
Mike Simmons
Ranch Hand

Joined: Mar 05, 2008
Posts: 2969
    
    9
Aj Prieto wrote:As for the if-else statements, you could use "switch" statements, which would look nicer, in my opinion.

I disagree about it looking nicer, but more importantly, switch statements will not work at all for inequalities such as those shown here.

Aj Prieto wrote:You should probably look at the cases in your if-else statements. Specifically, what would happen if annualSales were 120,000.00.

Or, what if it were 96000? There are two different problems there, as the inequalities don't quite match up the way we'd want.

More generally, you can avoid these misaligned conditions but stating the conditions in a less redundant manner. For example, the following code:

can be replaced with:
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 36514
    
  16
The problem about the nice numbered format is that it preserves those long lines. Long lines and lots of blank lines make the code more difficult to read. I have broken the long lines and you can see how you ought to do it I have not removed the blank lines however.

You are also inconsistent about magic numbers. You correctly used SALES_TARGET as a constant and then wrote 120000 elsewhere. You ought not to use the 120000. You are using // comments for methods constants etc. Not only do those make the lines too long, but also you ought to have used /**...*/ comments, which create documentation.
Have you been taught about the % tags in printf? Never use \n unless you have been told the LF character is specifically required. Use the %n tag instead, which gives the correct combination for your operating system. Never use "\n"; System.out.println(); provides the platform‑specific line end combination. Similarly don’t use the percent character. Use a % tag, and you can look here to find out what the correct tag is (hint: try %%).
Never use floating‑point arithmetic for money. It is prone to nasty errors. One day you will enter something which works out as 96000.01 and the JVM will calculate it as 96000.0099999..., or something similar will happen. In this thread, the OP thought he had a value of 0 and it was actually 2⁻⁵¹. If you manage to get 96000.0099999...., your if block will fail to work correctly. Particularly if you use a combination of <= 96000 and >= 96000.01.
For money you should use
  • Integer arithmetic: good. Use whole numbers of $ or denominate the money in ¢s
  • BigDecimal: Better. Much better. Read this thread, and about compareTo instead of equals. if (bd1.compareTo(bd2) == 0)...

  • Your if block makes the main method very long. In my opinion a main method should be one statement long, and everything else should be moved to other methods.
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 36514
        
      16
    And I see other people have already made some of my points, only better.
    Daniel Flannigan
    Greenhorn

    Joined: Jan 22, 2013
    Posts: 9
    Aj Prieto wrote:

    Also, you shouldn't have to java on your own. You say that you're taking a class, why not ask the instructor or the other students for help? It might help you to understand things because you could get instant feedback.


    Typically yes, I would think that would be true, but this is not typical. I am in a programming class and everyone else has a few years of experience in java where I do not. Since their levels of understanding are beyond mine I have been left behind in trying to understand the fundamentals. To give you an example, the first assignments output was to return a few variables and your name. Everyone in the class built GUIs to show their levels of understanding. While I thought that was cool, I was left scratching my head. The instructor is nice but believes in the student searching out the information on their own. In short, I have to learn by myself. it is not a perfect situation but I am dealing with it the best I can. I mentioned before that I have built all of this by myself thus far, but there is so much information in there now that I really have no idea what to do next. I have had to start over twice now because I messed up and couldnt remember the code order so I am a little wary now of huge changes, I dont want to start over. The order of which the program runs and calling a method from a class to the main is what I think is the hardest thing to do and very confusing.
    Mike Simmons
    Ranch Hand

    Joined: Mar 05, 2008
    Posts: 2969
        
        9
    It's hard to read this and understand what's going on. I would break the main method into several smaller methods, with nice descriptive names on each so as to understand the goal of each piece of code.

    What's with the extra pair of braces on lines 64-65? Or the opening braces on lines 46 and 52, which are closed on lines 164 and 160 respectively. These don't do anything, other than cause confusion from people wondering what they do. I suggest removing them.

    Your indentation is also sometimes inconsistent, again causing confusion. E.g. lines 22, 27, 31, 32, 45, And there's no added indentation after the braces at 46 and 52, Making it very heard to tell what's happening. By the time we get to line 100-104, I have no idea what the code is supposed to be doing; the indentation appears to be jumping around randomly here. At 137, it mysteriously changes levels for no reason. And the line of } at the end suggests you don't have any idea either how many levels of braces are present - which we should have been able to tell from the indentation, if it were consistent.
    Daniel Flannigan
    Greenhorn

    Joined: Jan 22, 2013
    Posts: 9
    Mike Simmons wrote:It's hard to read this and understand what's going on. I would break the main method into several smaller methods, with nice descriptive names on each so as to understand the goal of each piece of code.


    I agree, but like what? I am an ultra beginner and this is the best I can do right now.

    Mike Simmons wrote:What's with the extra pair of braces on lines 64-65? Or the opening braces on lines 46 and 52, which are closed on lines 164 and 160 respectively. These don't do anything, other than cause confusion from people wondering what they do.


    If you are confused on how to make this code work imagine how I am. The extra braces at the end do indicate that I do not have them in the correct places, but when I move them around everything goes red with errors. I know it might not be best practice, but that is why I am here - to learn.
    Winston Gutkowski
    Bartender

    Joined: Mar 17, 2011
    Posts: 7064
        
      16

    Daniel Flannigan wrote:I agree, but like what? I am an ultra beginner and this is the best I can do right now.

    Then my advice is: StopCoding (←click).

    You will never solve a problem in Java if you can't explain it in English, so follow the advice in the link.

    It sounds to me like you're getting tied in knots because you're trying to think like a computer rather than like a human being with a problem to solve.

    Back up; have a coffee; take a time-out if you need to; and come back to it when you don't have all this code whizzing around in your head - and think about the problem, not the code.
    And whatever you do, don't write a line of Java until you're absolutely clear what you have to do, and can explain it in English.

    From what I can see, you already have several nicely named and well-written methods in your SalesCompensation class. You just need to do something similar for your SalesCompensationApp class to take all that dross out of your main() method.

    Winston

    Isn't it funny how there's always time and money enough to do it WRONG?
    Artlicles by Winston can be found here
    Daniel Flannigan
    Greenhorn

    Joined: Jan 22, 2013
    Posts: 9
    Here is the code that is revised and I am close. This is the kind of Java that I kinda know how to do thus far. The calculations work but for only one salesperson. The program output is at the bottom, where am I going wrong with the return tables to get the final product to be able to return both salepersons data? I am now stuck! This only returns the second salepersons input and calculations. Can someone show me exactly where I am wrong? Thank you.





    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 36514
        
      16
    Daniel Flannigan wrote:Here is the code that is revised and I am close. . . .

    }
    if(annualSales >= 96000 && annualSales < 119999.99)
    . . .
    if(annualSales >= 120000.01)
    . . .

    Jim's Annual Sales Commission Potential
    Total Sales Rate Commission Total Compensation
    ====================================================================
    $126000.00 16.25% $20475.00 $50475.00

    $131000.00 16.25% $21287.50 $51287.50

    $136000.00 16.25% $22100.00 $52100.00

    $141000.00 16.25% $22912.50 $52912.50

    $146000.00 16.25% $23725.00 $53725.00

    $151000.00 16.25% $24537.50 $54537.50

    $156000.00 16.25% $25350.00 $55350.00

    $161000.00 16.25% $26162.50 $56162.50

    $166000.00 16.25% $26975.00 $56975.00

    $171000.00 16.25% $27787.50 $57787.50

    $176000.00 16.25% $28600.00 $58600.00

    $181000.00 16.25% $29412.50 $59412.50

    $186000.00 16.25% $30225.00 $60225.00
    No, you are nowhere near close, I am afraid. You still have the long methods and the long lines. The output at the bottom suggests you are calling println too often. The two if lines demonstrate a serious logic error when you bring them together.
    I think you need to learn about else-if.
    You have marked some fields static. Why? I have a rule of thumb: anything marked static without a good explanation is a mistake.
    Daniel Flannigan
    Greenhorn

    Joined: Jan 22, 2013
    Posts: 9
    The problem with being a beginner to Java is that some of the suggestions that I am getting don't make any sense to me. They might be best practices and the absolute way to do them, but without seeing the suggestions in action I can't put the ideas together. My learning style is first see, then do. If I need Else-If statements, where? How would those work with what I have already? It was said I am using println too often, what is the alternative for a beginner that returns the same results? Frustrated.
    Winston Gutkowski
    Bartender

    Joined: Mar 17, 2011
    Posts: 7064
        
      16

    Daniel Flannigan wrote:The problem with being a beginner to Java is that some of the suggestions that I am getting don't make any sense to me.

    And I suspect that's partly because
    (a) You're not listening to good advice, of which the first item is DontWriteLongLines (←click).
    Your last set of code contains reams of them, and they need to be broken up. You can do that via the 'Edit' button.
    For one thing, it makes your code difficult to read, and for another, it screws up the windowing here, making your posts difficult to follow.
    Fix that first.

    (b) You're tackling far too much at once. Break your problem up into individual pieces and tackle them one at a time.
    And don't continue on to the next piece, until you've compiled the code for the first AND TESTED THAT IT WORKS.

    My learning style is first see, then do.

    Fine, but nobody here is going to simply hand you an answer. If you want help, focus on a specific problem and ask a direct question;
    don't just hand us an enormous pile of code and say "What's wrong with this?"

    Winston
    Daniel Flannigan
    Greenhorn

    Joined: Jan 22, 2013
    Posts: 9
    OK! I have it almost done, (I Think). I need a second set of eyes to please tell me where my calculations are going crazy about 96000 Annual Sales input. When I input anything below 95,999.99 my commission rate is 12.50%, the way it should be. That works great. But when my input for Annual Sales goes over 95,999.99 my commission rate shoots up over 100,000%! Right at line 70 and 71, BOOM! I have looked at this forever and tried to debug it (the best I knew how).. Nothing. So, I have come all this way, can someone please look at this and tell me where my commission rates are going crazy at? Thank you so much.



    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 36514
        
      16
    Daniel Flannigan wrote: . . . I need a second set of eyes to please tell me where my calculations are going crazy . . .
    You will have to create those eyes yourself. Start by reformatting all that code. Indent it correctly as suggested here. Your } are farther to the right than your {. If you indent everything correctly, you will be able to see the structure of the program just by staring at the code. It becomes obvious, even without reading anything. Put one blank line before every method, too. And for the umpteenth time, break those long lines. I think I showed you how to do it, in your first post.
    Create the eyes by putting print statements in each limb of the multiple if blocks. That will allow you to follow execution and see what the values are at different places in your code. I have already told you about severe logic errors, and given you a hint where you would find them. But, as Winston hints, you appear not to have read what I said.
    Daniel Flannigan
    Greenhorn

    Joined: Jan 22, 2013
    Posts: 9
    I spent a lot of time reworking the code the best of my understanding, a few of those links helped too, Sheriff. I worked a lot on it today and it still doesnt work in the percentages. It was turned in tonight not working fully. The commission returns are still all wrong. I do respect everyone's input here, thank you very much. Hopefully the instructor can identify what went wrong. If he gives me the feedback I will post it here so everyone can see what the problem was and how it is supposed to work. Thanks again!
    Campbell Ritchie
    Sheriff

    Joined: Oct 13, 2005
    Posts: 36514
        
      16
    I have already told you what was wrong. Or more precisely where to look for it. We have told you several times there are other things wrong, including the dreadful formatting.
    Winston Gutkowski
    Bartender

    Joined: Mar 17, 2011
    Posts: 7064
        
      16

    Daniel Flannigan wrote:I spent a lot of time reworking the code the best of my understanding...

    Yes, but you continue to ignore our requests to break up those enormous lines; as a result of which
    your thread is becoming more and more unreadable. And you need to do it in ALL your posts.

    Until you do so, you can expect no more help from me.

    Winston
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: Ugly code and stuck
     
    Similar Threads
    Requiring boolean...
    serializing java codes
    Can someone help me get my Table.java file to work with my AnnualCompensation.java file?
    Salesperson array
    Java method to output difference between two numbers as a comparison showing the highest number