tangara goh wrote:Hi guys,
I would like to have someone review my code.
The objective of the code is to encode a source String with a character that is indicated and the source String will be used as the reference table. So, A will be 0, B will be 1, C will be 2 and D will be 3 till end of the reference table which is String source.
I am now using a hardcoded 1 as an index to generate the rotatedArray - the target array for me to populate the output : BGDKKN VNQKC
Thanks a million.
Campbell Ritchie wrote:Please tell us the algorithm you are using. It looks like a Caesar cipher with negative changes..
Why have you made so many things static? Aren't you using object orientation? Why is line 72 calling static methods on an object reference?
Your lines are too long and the // comments make them longer still. Some of those comments should probably be changed to documentation comments.
Why have you not made the fields private? Come to think of it, you should most probably have made most of those methods private, too.
What is positionToUse() supposed to do? the comment is by no means clear, and the method appears to have local variables and loops which don't actually do anything.
What is offset, and why might it equal a source character in line 65? Don't start the name of that method with a capital R. It also seems to contain code repeated from another method, which should be refactored. Or maybe deleted.
Don't present code for review with parts commented out. I found something commented out that I don't like: arithmetic with Math.random(). Use a Random object instead.
What do you mean about had‑coding values? You appear to have presented us with incomplete code.
tangara goh wrote:I have tried to edit my original but was not able to
That's a pleasuretangara goh wrote:. . . Thanks a million.
I think probably no. I think you have lots of code you are not using and lots of algorithms you are not using. It looks like code that had just growed (Topsy: in Uncle Tom's Cabin. . . @Campbell,
Should I shift the static String to the method . . .
What does it do? Where are you using it? I don't think you need a position at all. You want an input and an output. Decide whether you are doing it in a functional or object‑oriented way. Decide what to do about spaces. Decide the arithmetic to convert A to Z for the cipher. I think you will end up with the code occupying about 15% of the space it does at present.I am not sure if the positionToUse is necessary . . .
Campbell Ritchie wrote:What does it do? Where are you using it? I don't think you need a position at all. You want an input and an output. Decide whether you are doing it in a functional or object‑oriented way. Decide what to do about spaces. Decide the arithmetic to convert A to Z for the cipher. I think you will end up with the code occupying about 15% of the space it does at present.
tangara goh wrote:The assignment stated that they want OOP design....
What is a functional design ? I am not clear about it? Is the code I have posted here OOP or functional ?
Education won't help those who are proudly and willfully ignorant. They'll literally rather die before changing.
Paul Clapham wrote:
tangara goh wrote:The assignment stated that they want OOP design....
What is a functional design ? I am not clear about it? Is the code I have posted here OOP or functional ?
It isn't OOP because you haven't used any objects (except in the most trivial way). It isn't functional because you haven't used any functional programming features.
Although for this assignment I would have used code similar to what you posted. I suppose it would be possible to design some objects to do this character-mashing work but I wouldn't ask people I was considering hiring to do that. It might just encourage them to do it when working at my organization.
Education won't help those who are proudly and willfully ignorant. They'll literally rather die before changing.
Also find the mistake I inserted late in the process and correct it.A few minutes ago, I wrote:. . . Now work out . . .
Campbell Ritchie wrote:Now work out how you can use other classes representing [edit: complete the sentence] different algorithms. Work out which checks I have written and which not written and why.
There is a construct that allows you to create objects of a class with all those parts added automatically.tangara goh wrote:. . . Why don't Java built in this part . . .
Education won't help those who are proudly and willfully ignorant. They'll literally rather die before changing.
Knocked something together in a couple of minutes over a drink.Tim Holloway wrote:ACK! Campbell, WHAT HAVE YOU DONE??? . . .
I have already explained equals(). You always need toString(). The short way of creating such a class would have included auto‑generated methods like that. I am keeping quiet about what the short technique is.Also a lot of rendundant code . . . .
Campbell Ritchie wrote:
Knocked something together in a couple of minutes over a drink.Tim Holloway wrote:ACK! Campbell, WHAT HAVE YOU DONE??? . . .
If you make the fields final, you have to initialise them before the constructor completes.I have already explained equals(). You always need toString(). The short way of creating such a class would have included auto‑generated methods like that. I am keeping quiet about what the short technique is.Also a lot of rendundant code . . . .
As an alternative, you can write the whole thing in a functional style.
Education won't help those who are proudly and willfully ignorant. They'll literally rather die before changing.
I didn't write any. I should probably have written a test with a regex to confirm that the input only contains the twenty‑six English letters and spaces and maybe punctuation. I should probably have written a test whether the difference was in the range (int)'A'...(int)'Z' or similar. I could have written a test for whether the input is null, but thought I would suffer an exception as soon as I tried to manipulate that String, so it would be found out very soon.Last week, I wrote:Work out which checks I have written and which not written and why.
Campbell Ritchie wrote:It takes more than that to get me under the table. The true object‑oriented approach would want one object per message.
Education won't help those who are proudly and willfully ignorant. They'll literally rather die before changing.
This guy is skipping without a rope. At least, that's what this tiny ad said:
Gift giving made easy with the permaculture playing cards
https://coderanch.com/t/777758/Gift-giving-easy-permaculture-playing
|