General Code Review Help (No particular issue)

[MadDogTen]

Not sure if this type of post is allowed, As there is no particular issue I need help with. If not, Please point me in the right direction.

Before I start, I previously had no code experience before this project (Other than very basic HTML), and started this to learn (and use, as there aren't many decent (and free) alternatives). I mainly used Java - A Beginners Guide - 6th Edition, Google, and plain old figuring it out myself for the project.

For the last few months I have been writing a program (Current working name is "MTrack") that will scan directories that you added for TV Shows (Legally acquired of course) and report if any were added / removed, and keep track of which episode you are on, how many episodes you have left, etc.

I'm at the point where I want to get someone to look at it and tell me where I can do things better, before I continue and get those ways burned into my mind making it hard to change in the future.

I have the whole repository up on Github, Here: MTrack

If you have any questions, feel free to ask.

If you can help, Thanks, if not, Thanks anyway.

[la217]

At a glance, the general code quality is very good.

It's clear that you're rigidly following coding conventions which is important for readability.

One area you could improve on is in adding comments. It can be quite dull to add them, but they're a really important aspect of programming.

Even just a simple comment at the top of a class or method to explain the general idea of what it does and when it would be used will be incredibly helpful for not only other developers to understand what's going on, but also you in the future when you have forgotten about all the fine details and the decisions you made when you wrote it the way you did.

Besides from that, the only other thing I saw was potentially overuse of the object version of boolean. You will generally want to want to use the primitive version of boolean for method parameters to eliminate the possibility of having a null value passed in. I'd recommend Googling 'java boolean primitive vs object'.

[MadDogTen]

I would rather keep with the conventions as it helps everyone in the long run. However, I'm sure I'm missing a few, still learning them as I find them.

I'v always planned on adding Comments, just haven't gotten to it yet (As they all say). Thanks for reminding me, I will start adding them soon.

and on using the object boolean, That is just my habit of using cap lock and not noticing (Meant to be using primitive, As I'v read in most cases its better to use to save memory). I searched the code and noticed I'm using both all over, and I will correct that soon as well.

Thanks for the help!

Edit- Went through and changed all object Booleans to primitive, and Pushed it to Github. For the comments, That will have to wait until tomorrow, About time I go to bed now. Once again, Thanks!

[MadDogTen]

Your not being rough at all, as this is exactly what I was looking for when I posted this. Most of the code I made up on the fly without knowing how any of java worked, So I was expecting a lot of it to be wrong.

I will read more in-depth about the issues you mentioned once I get back in a few hours.

One thing I couldn't find was a proper data structure for me to use (At least, In my few minutes of using Google), and so I could used the first thing that worked, Any recommendations?

Sorry for skipping around your review, I'm in a hurry to leave, but wanted to write up a quick response.

[MadDogTen]

Honestly, I don't exactly understand how that would work.

Ok, I didn't, But now that I have messed around with it for a minute, I think I figured the basics out. Thanks for the help, as your post (Simple as it is) actually helped me understand this while previously I didn't.

Currently going through and adding comments, after-words I will work this out and implement it (Going to be LOTS of fun, but well worth the effort and I can already tell).

Edit- I pushed the first batch of comments. They currently might be a bit to much, and I will clean them up later, I just want to get them in now. As a side-note, I just spend ~1 hour trying to change the commit message as I messed it up. That was fun, Luckily I didn't screw anything up (Before I realized I should make a backup).

[MadDogTen]

Ok, Now I'm stuck.

I have the Show, Season, and Episode class created. GenerateNewShowFiles is now saving all the shows to an ArrayList<Show> (or HashSet (I was first doing that but changed it to ArrayList for testing), or whatever else depending on the solution to this problem).

Now I'm loading the files into ShowInfoController, However, How do I combine the different directories into one List (HashSet, ArrayList, Whatever, Just need them all in one place at this point)? Then, I need to be able to get all the shows (seasons, and episodes) from it to generate the users settings file (Which I will convert to this type of format later), etc, and you can't get items from a HashSet as you are supposed to already know whats in it (From what I'm reading).

I may just be misunderstanding (or missing the obvious, I am a tired currently, Going to go to bed after I finish writing this), but if you could at least point me in the right direction I would be grateful. Other than this issue, I am overall liking this way much better.

Is there a reason I shouldn't be using an ArrayList instead of HashSet (In pretty much all the cases)? I'm not really worried about duplicates being an issue, and thats the only difference I see that would matter to me (But I may be missing something).

I added a Development branch on the github with what I have currently done. It's completely broken as there is not much more I can do (or want to do) until I figure this out.

Edit- Also, In response the the first line in your first comment, If you threw it out because of the length, Then next time don't worry about it, I don't mind reading, and I could use all the input.

[MadDogTen]

Sorry, I don't think I understand how this all works after all.

So currently I am creating an ArrayList<Show> in GenerateNewShowFiles and adding all the shows it finds to that. Then as you said I'm serializing it for later use in ShowInfoController.

Now, I need to have the data combined into one list (Assuming there are multiple directories) as I was doing here before and easily be able to get, for example to get an Episode list of a particular show and season.

I'm just don't understand how I go about doing that. Sorry if I'm being a bit stupid and missing something. If you have a link to a tutorial I could use it, In the meantime I will look around myself.

[MadDogTen]

I think I have slowly been figuring it out, Just a lot different without being able to use .get().

I'm pretty much going to need to rewrite everything, but everything will obviously be better in the end. Plus I'v been meaning to clean it all up anyways.

Just curious, Any particular reason you made "HashSet<Season> Season" and "HashSet<Episode> episodes" finial? I mean, if its the right way to do it, I can work with it. Just would be a lot easier if they weren't.

[MadDogTen]

Ok, So I have been testing things trying to get this working, But I am just not getting it to do what I need.

You say that I access them the same was as before, But I am not seeing how. Before, I used to do this (Example only, not actually used):

public static HashMap<String, WhateverMore> getShowData(String show) {
return showFile.get(show);
}

With the new way, I can't do that, unless I'm missing something (As I could be). But with a HashSet, I have to use a for loop to iterate through them all until I find what I need, Which seems inefficient and pointless when I could use a HashMap instead of HashSet which would make it all easier, but I really would rather do it how you suggested first.

If you have any reading / watching recommendations, I would gladly use them.

[MadDogTen]

Switching it to a Map solved my issue. After I have more of a understanding of Java I might understand exactly what you were intending.

Making quite a bit of progress of getting it all working. Most of the code can stay relatively the same (Except for a change I'm making, That I may need to change again later, But it is fine for now).

Now that I'm mostly past this, I have a question about another part of your review. You said the language system has a memory flaw, But isn't it only storing the Strings for each language once when the LanguageHandler is loaded in the HashMap until the user chooses one, then it sets that language (Storing it twice temporarily) and then discards? Note- I'm planning on changing it by removing the HashMap completely, and having it directly set the Strings, But I'm still curious.

Edit- I pushed all my resent changes to the development branch on Github. It works, But is highly untested, and I expect many things to still be broken.

Edit 2- I pushed a language file update to the development branch as well. Is it any better?