• Post Reply Bookmark Topic Watch Topic
  • New Topic

problem finding a string in an array  RSS feed

 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
i am having trouble currently, i know there will be other bugs, but my current problem is when im trying to choose case 2 and call markItemOnLoan i cant seem to find the title i want after i enter a few.
i will include all of my code but i have mentioned what im currently stuck on



 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Don't write such long lines; I have broken up the worst offenders so you can see how to do it. I have also deleted the excess blank lines which do nothing for eradability.
Why have you got a MediaItem() constructor which does nothing? Delete that.
Don't write == true or == false. They are both poor style and error‑prone if you write = by mistake. Simply delete them and replace any falses with bang signs (=!).
That main method is far too long. Partially because you have all that repeated code in each case. Each case can simply call a method.
Variable names like mCall and call are very poor. Not quite as bad as count, but not far behind.
How are you finding a media item from its title?
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Kip Bodey wrote:i am having trouble currently, i know there will be other bugs, but my current problem is when im trying to choose case 2 and call markItemOnLoan i cant seem to find the title i want after i enter a few.

How do you know? What are the symptoms? Do you, for example, get an error message?

i will include all of my code but i have mentioned what im currently stuck on

It's good that you did, because it gives us what we need to (hopefully) help you. Well done. +1.

And the main reason for your problem is that In your markItemOnLoan() method you call
if (items[i].equals(title)) { ...
yet your MediaItem class does not define an equals() method (items[i] is a MediaItem remember).

That means it will use Object's equals() method, which simply checks if two objects are the same object or not.

However, your problems don't end there.

As previously stated, items[i] is a MediaItem. But 'title' is a String. How do you propose to compare a MediaItem with a String? You could do it, but I'd advise against it. STRONGLY.


My advice: StopCoding (←click).

It looks to me as if you've bashed out a lot of code "existentially", without much of an overall plan, and you're now reaping the rewards. Think about WHAT you want to do, and write out the steps you need to follow to achieve it. In English.

It may mean, unfortunately, that you have to start again; but at least then you'll be designing instead of just spewing out code.

And don't worry, it's a common problem: We've ALL done it (me especially).


And a few other things for you:

1. You're making life difficult for yourself by using an array for your items. A List (eg, an ArrayList) would be much better - specifically, an ArrayList<MediaItem>. And if you're not familiar with Lists, at the very least you should check in your addNewItem() method whether you CAN add a new "item" or not - ie, are you trying to add the 101st one?

2. Make your class fields private. ALWAYS. No 'if's or 'but's: ALWAYS. And if a method isn't used outside your class, make those private too. The "default" visibility is almost never needed, so don't use it - at least not until you know why you would. For now, methods should be either private or public; and DON'T make them public unless you know you need to.

3. Get rid of all those setters - especially in your MediaItem class. And, as Campbell says, that "no-args" constructor too. Think about it: can a MediaItem title change after it's been created? Or its format? I suspect not. And all those other fields: aren't they simply part of a "loan" operation? Right now, you can (and do) change anything you like, and you do it externally. A class should be responsible for everything to do with itself, and in the case of your MediaItem class, that includes "loaning itself out", so my advice would be to add a 'loanOut()' method that takes a date and a "loanedTo" value, instead of all those setters.

4. Don't use Strings for everything - especially dates. A String is NOT a Date, and you're going to tie yourself in knots if you try to use them as though they are. For more information, have a look at the StringsAreBad page.

5. Since searching your items array is likely to be something you want to do a lot, why not split that out into a separate method? viz:and then your other method might look likeDoesn't that look better?

HIH

Winston
 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
everything you told me not to do, are things i am doing because my professor told me to do. I'm not saying I would have done them differently but, lol my teacher did say to do it this way. every one of my methods must be included in my project. All of the fields i have also must be included in my project. Everything must have a setter and getter per my professor. The private and public and everything... She has said to do it how she says. This is my i think 13th try on this program. ive spent days thinking. Yesterday however i solved a couple of of the problems i had been working on and I think i got ahead of myself. but thank both of you very much for your help. I will take another shot at it today.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Kip Bodey wrote:everything you told me not to do, are things i am doing because my professor told me to do.

Fair enough. I don't like the approach, but if you've been told to do it...fine.

Unfortunately, that leaves us in a bit of a quandary, because we now don't know what you've told to do, and what not to.

This is my i think 13th try on this program. ive spent days thinking. Yesterday however i solved a couple of of the problems i had been working on and I think i got ahead of myself. but thank both of you very much for your help. I will take another shot at it today.

Welcome to programming.

Did you read the StopCoding page? Because I think that your main problem right now is that you think you can solve this by coding - or by trying out different permutations of code. And you're getting yourself tied up in knots because you're having to deal with what you've written.

Well, take it from an old soldier - it won't work.

That sort of approach is fine when you're dealing with "Hello World"; but it doesn't translate into the real world where you have to solve problems. You need to understand WHAT you want to do before you write one line of Java code. Your problem is that you appear to have written hundreds; and ours is that we don't know what of that was given to you (or mandated by your prof) and what wasn't.

I'll say it again: StopCoding.

Right now, you're simply adding to the mess (and I hate to say, but at the moment it is a bit of a mess).

Chill out. Have a beer (if you're old enough) or go play some footie or a video game and forget about it. Or just sleep on it. Then come back, armed with a notepad, lots of pencils, and a sharpener. And THINK. Don't code ... THINK.

Put some chicken wire over your keyboard if you have to - or even (horrors!) TURN YOUR COMPUTER OFF.

And don't turn it back on until you understand the problem, and can describe it in English and in DETAIL.

The keyboard is not your saviour. Nor is Java, or your computer, or your IDE (if you use one). It's YOU, and your brain; so stop trying to "think in Java".

At the level of this problem (and this is not a put-down) if you can't explain WHAT you want to do, in English, to a 12-year old with no knowledge of Java, then you WON'T write a good program. You might be able to get what you've written to work; but that's not the same thing.

And feel free to describe it to us if you want. That's what we're here for, and we'll be happy to steer you in the right direction, or point out errors - but the thinking has to come from you.

And, as I said, don't worry too much - we've all been there. It just takes practise. What we may be able to do is save your nice flatscreen a few dents.

HIH

Winston
 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Again thank you I did read that page, twice actually. My teacher had us read one very similar. I have been working on this project method by method. I have fixed my last problem, I am now working through some of the things it is doing the i dont want it to do. For instance throwing an error message, but instead of not doing it, it did it to my next index. LOL but i am on my way to a better project. I am confident I can figure the rest out on my own, but if not I am sure I will see you soon. Thank you again for everything.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Kip Bodey wrote:Again thank you I did read that page, twice actually. My teacher had us read one very similar. I have been working on this project method by method.

A very good approach. Deal with one thing at a time. You'd be amazed at how many experienced programmers don't remember it.

I have fixed my last problem, I am now working through some of the things it is doing the i dont want it to do. For instance throwing an error message, but instead of not doing it, it did it to my next index.

Tip: System.out.println() is your friend.

but i am on my way to a better project.

Sounds like it. Next time, try to remember: Don't code (not even ONE line) until you know - or at least until you have a darn good plan.

I am confident I can figure the rest out on my own, but if not I am sure I will see you soon. Thank you again for everything.

You're most welcome. And good luck.

Winston
 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
okay maybe i actually do need something. here is my english for this problem. for each item in the items array if the title is equal to the title then call markonloan if it doesnt equal a title print error message. it is printing an error message no matter what
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Kip Bodey wrote:okay maybe i actually do need something. here is my english for this problem. for each item in the items array if the title is equal to the title then call markonloan if it doesnt equal a title print error message. it is printing an error message no matter what

Right. So back up. You want to find a MediaItem with the same title.

What's to stop you putting 13 MediaItems with the same title in your array? And what if the FIRST one you find has already been "loaned out"?

This is what I mean about making a plan.

However, that is NOT your problem in this case. Look at the code:For EVERY item you are doing your check, and then throwing an error if it doesn't hold true, which means that if ANY item doesn't match what you're looking for, you'll throw an error.

NOW: What do you THINK you should do? Believe me, the solution is right there in front of you; but you've GOT to StopCoding.

Winston
 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
my teacher didnt say anything about it mattering if we entered duplicate titles im assuming she doesnt care at this point in our journey. again im trying to look at this with out opening eclipse. ive even drawn a picture and it seems to me as if i would want to have an if statement outside of my loop. so if after searching you find it check it out. however if you dont find it you cant check it out. is that the right idea?


edit: i actually did that on a whim it appears to work so far, i am going to test it a little bit, but the code looks nasty to me.

still curious if thats what you had in mind?

ill post my code after i test it some more
 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
this works ive tested four different ways and orders to make sure it all goes correct, but i think its ugly or lol "codey" like "wordey" im not sure what do you think?





edit: actually it isnt working. oh my gosh i am sorry. it isn't throwing that error message at all now

 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
thank you for your time i finally got it, im going to post my code, just in case someone else finds this helpful
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Kip Bodey wrote:thank you for your time i finally got it

Well done! I knew you'd get there. Now just make sure that you put that same thought into all the other code you write.

And have a cow for posting your solution.

Winston

PS: What you've got will certainly find an item; but what if it's already loaned out?
 
Kip Bodey
Ranch Hand
Posts: 43
1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have another method modeled just like this, that does the exact same thing and calls another method to check it out as well.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!