I'd like to help, but I'm a little puzzled as to what you want to do, exactly. I assume the code you have shown us is all inside a "main" method, but what do you want your new method to do? Just moving the creation of a variable into a method seems hardly worth the effort. Do you plan to move any of the code which uses that variable there too ? For a "main" method your code doesn't actually seem to do much. It just seems to copy the contents of a 2D array to a 1D one. What's this for? I have a feeling that maybe the whole conversion process should be in a method, which takes the 2D array as a parameter, and returns the 1D array. Without more idea of what you are trying to do, that may be way off, though.
What's this for? I have a feeling that maybe the whole conversion process should be in a method, which takes the 2D array as a parameter, and returns the 1D array. Without more idea of what you are trying to do, that may be way off, though.
This is only portion of the mian method. What i am doing is taking the 2D array, chopping off the first column and the creating a 1D array of the rest. The first column is an ID and the remaining columns are data. Moving that entire conversion process into a method is exactly what I am trying to do. Thanks, Paul
What specific trouble are you having with this? Show us some code that isn't quite doing what you want it to do, and we can diagnose it.
Ron Newman - SCJP 1.2 (100%, 7 August 2002)
posted 16 years ago
This is only portion of the main method. In which case your main method is already way too big. We'll deal with getting this processing out of the way first, but then I strongly recommend splitting up the rest of it, too. What i am doing is taking the 2D array, chopping off the first column and the creating a 1D array of the rest. The first column is an ID and the remaining columns are data. OK, so far, so good. Moving that entire conversion process into a method is exactly what I am trying to do. Excellent. Can I assume that your program "works" at the moment? If so, we now just have some refactoring to do. In case you are unfamiliar with the term, "refactoring" means changing a program internally, without changing its external behaviour. The trick to refactoring is to do it in very small steps, each of which you are sure you know how to do. Never "bite off more than you can chew". If we ignore the rest of your program for the moment, and treat the section you gave as if it were the whole main method, we have something like:
It looks a lot like you are mixing your test data (that 2D array) with your code to process it. In general, this is a bad thing - you might inadvertently deliver test code to a user, and it makes understanding and refactoring the code much more complicated than it need be. For the future, I strongly recommend learning and using the JUnit test framework to build your tests. Without getting into separate test harnesses for the moment, lets address the first issue, and move the test data away from the processing:
That's a bit better. Really, though, if we want the processing to work for any supplied 2D array, it has to be a parameter passed in to a method. That's where we hit the second problem. You can't pass parameters into "main". So, lets start by just extracting the processing as-is into another method, and worry about parameters later. One thing we have to be careful about is the "do some stuff ..." bit. Somehow we need to make justGrades available to the following code. Ths simplest way (for the moment) is to make it a static member variable, the same as "grades". Don't worry, we'll sort it out later.
Now, a quick diversion. If you wanted to, at this stage, you could stop here. To process a different set of grades, just set "grades" to a new 2D array, and call "convert2Dto1D" again. I don't want to go into too much depth about this, but although superficially attractive, this could be a dangerous solution. It's easy for a user to get wrong (set the grades, but forget to call the method; keep a reference to "grades" or "justGrades" somewhere else, and be surprised when they change "behind the scenes" and so on). It's well worth pushing on a bit to make this a proper method which takes parameters and returns a value. Such as:
This is looking much better. For your information and interest, a lot of the common refactorings have names. The one we've just done is known as "extract method". I recommend the book "Refactoring" by Martin Fowler for more information on the topic. I don't know what you code which uses "justGrades" looks like, so I shall just assume you can do the same trick with that, like so:
Now, I can't help noticing that there is a bug lurking in here. If you have anything other than 3 student records, the calculation of "length" will either throw an exception (if fewer than 3) or get the wrong answer (if more). Interestingly enough, this is also part of a "code smell" :- the "grades[..].length-1" bit of code is repeated four times in just this small bit of code. A good thing to aim for when refactoring is that all duplication is removed, so how can we remove this? First, I'd make a small "helper" method to do this calculation:
Is this useful? It seems to have actually made the code bigger ! The trick here is that it's also more readable, and more flexible for the next steps. We've still got the duplication. Sorting out the three in the calculation of "length" just needs a "for" loop:
By doing that we've both fixed the bug and removed (most of) the duplication. Here's a reminder of where we are so far:
I'm still seeing some duplication, though. It's starting to become more and more irritating to pass in "grades" to every little method. How can we avoid that? This is quite a big conceptual step, but the end aim is that we start using real OO (Object Oriented) programming, and create an object. So far we have only been working with static methods in a class, as that's all we have needed. Remember, though. small steps, and keep it working ... The first step is actually to undo something I did at the start, and move "grades" back into "main" ! Don't be afraid to do this sort of thing when refactoring. Moving it out helped us to understand what the code needed at the time, and now it's much "safer" to bring it back into main :- there's no real code left in there!
Now, to share "grades" between all the methods, we need to make it a "member variable". The easiest way to get one of those is to create a "constructor" so you can give it to the object when you create the object:
How do we use this ? Well, we start by calling this constructor to create an object in "main": As an aside, I'll rename the "grades" array in "main" to "testgrades" both to make its intent more clear and to prevent confusion with the new "member variable" called grades.
OK, we now have an object. Now we can go through each of the methods and and method calls and (a) remove the "static" and (b) remove the "grades" parameter so it can use the member variable instead. We're nearly there, now. All we need to do is to make the calls to each of the methods in "main" refer to the object we've just created. So the class now looks like:
There are no doubt more refactorings which can be done; There is a chunk of code which we didn't include in this process, I don't like those "<= something-1" expressions, I'd move the calculation of the length to its own method and so on. However, I shall leave those as an excercise for the interested reader. The class we've got to here is already much more amenable to change than it was at the start. I hope this has been helpful.
The soul is dyed the color of its thoughts. Think only on those things that are in line with your principles and can bear the light of day. The content of your character is your choice. Day by day, what you do is who you become. Your integrity is your destiny - it is the light that guides your way. - Heraclitus
posted 16 years ago
Frank, Thanks for the detailed response. It will solve my problem AND teach me a lot in the process. What more could I want! Paul
Of course, I found a very beautiful couch. Definitely. And this tiny ad: