Win a copy of Securing DevOps this week in the Security forum!

Junilu Lacar

Sheriff
+ Follow
since Feb 26, 2001
Junilu likes ...
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
Forum Moderator
Junilu Lacar currently moderates these forums:
Columbus OH
Cows and Likes
Cows
Total received
191
In last 30 days
4
Total given
65
Likes
Total received
1570
Received in last 30 days
13
Total given
291
Given in last 30 days
2
Forums and Threads
Scavenger Hunt
expand Rancher Scavenger Hunt
expand Ranch Hand Scavenger Hunt
expand Greenhorn Scavenger Hunt

Recent posts by Junilu Lacar

I think you've reached a point where this program is big enough that it requires some attention to design details.  All the code examples you've been given in this thread, including the one I gave, is far from ideal. They are examples, yes, but frankly, they are poor examples and I think it's a bit of a disservice to you to just leave you to figure that out for yourself.

You really need to step back and think about organizing different responsibilities and assigning them appropriately to classes or objects. This goes beyond just providing some kind of authentication functionality using hashes and PINs. Guiding you down that path would be quite a journey though, I think, but I just wanted to point this out before you go any further. The last thing I'd want to see is you "learning" a lot of bad habits and saying that's what the folks at CodeRanch taught you.
6 hours ago
If you want to learn about more secure hash algorithms, check this out: http://www.baeldung.com/sha-256-hashing-java
9 hours ago
Note that having all static methods is probably not the best thing in this type of program but if you haven't touched on object-orientation yet, that's fine for now.
9 hours ago
You shouldn't hard code the MD5 value in your code like you're doing on line 18. As I mentioned in another thread, you should keep the "secret" in a variable that's either a class variable or an instance variable. In this case, it looks like a class variable is appropriate.

For example:

At some appropriate point in your program, you'd call the changePIN() method and you can do this even when the user is logged in.
9 hours ago
Strategy relies on polymorphism to work. You can't make polymorphic calls to a static method so your choice to use static methods goes against the pattern.
2 days ago

Campbell Ritchie wrote:What about this?I think the old version breaches the suggestion of Winston's that Strings Are Bad, and including the status in the Message object is more object‑oriented.


I suppose that could be a design you could eventually refactor to. Refactoring should be done in small steps though. I could easily see the author's version being the first step, then you detect the smells that I pointed out and refactor to what I suggested. If that still seems smelly, you could certainly reify (message text + classification) into a Message class. If you want to take it a step even further, I'd make sure that the log method is the last publicly accessible API method and that writeMessage is a private method, at which point, I'd probably refactor to:
4 days ago
These kinds of discussions between experienced people can also be helpful for students trying to develop critical thinking skills. Please consider my idea of having a forum at CodeRanch specifically for code reviews that you can have your students participate in and get input from others.
4 days ago
Here's what I have seen in practice:

1. Examples in documentation either in the form of JavaDocs or a wiki page can be very helpful if they are up-to-date.

2. Busy developers seldom have time to keep documentation comments up-to-date and they rarely do. Take examples from documentation with a grain of salt and always try them yourself to be sure they work as advertised.

3. If you're writing a utility or library for general use, there's more incentive to give examples in documentation comments and keep them current. I trust those kind of comments/examples more than I do other kinds of documented examples. If you have them, that's great.

4. If developers follow naming conventions and project structure and directory layout conventions, you can usually find tests related to a class fairly quickly. I always look for a src/test/java folder because most projects I work with use Maven and the layout is pretty much a de facto standard. IDEs can also do a lot of the heavy lifting for you by finding the references to a method, which are in turn organized and sorted by location in the project structure.
4 days ago
Welcome to the Ranch!

Please cite your source for this. What book or software is this from?

Also, you could write some code to see which of these would work. Have you tried that?
4 days ago

I wrote:...
3. Don't complicate things prematurely.

To point #3 above, I would have suggested using a Map<Sting, String> as a start.
...
If your goal is just to learn how to store something for later use, then all you really need is to create a variable of the correct type in an appropriate scope, in this case, probably as an instance variable.


In fact, even a Map<String, String> might be overkill. You could just declare a String as an instance or class variable. It looks like you're still writing static methods so if you want to stick with that approach, then use a class variable.
4 days ago
A few comments about this entire thread:
1. MD5 is not secure - it is "essentially cryptographically broken"
2. A basic tenet of secure coding is to never create your own cryptographic function.
3. Don't complicate things prematurely.

To point #3 above, I would have suggested using a Map<Sting, String> as a start. Why would you need to go through all the gyrations of converting from String to int and vice versa anyway? As already shown, you can still validate input as a String so what's the point of converting to int?

As for security concerns, you should analyze your program's attack surface to understand exacltly what your vulnerabilities are and what risks your exposure present. Efforts to secure code have to be prioritized according to risks and rewards. I don't know if the complexity you introduce to your program here is even worth the effort, given that this appears to be a hobby project at best. Are these real PINs that would result in significant losses if they were to be compromised? Is this a real-world program?

If your goal is just to learn how to store something for later use, then all you really need is to create a variable of the correct type in an appropriate scope, in this case, probably as an instance variable.
4 days ago

Simon Harrer wrote:
Regarding your other code refactoring: hm, we should have named that boolean variable "useCaptainsLog" or something like that instead. Thanks for the hint. We'll try to get that fixed in the next edition. :-)


I disagree. The classified name makes more sense when you consider the values of CAPTAIN_LOG and CREW_LOG as implementation details. It was a good thing to hide those implementation details. Clients of the API could probably make a determination whether the message they wanted to log was classified or unclassified. In contrast, useCaptainsLog seems less intuitive. Why would you use the captain's log instead of the crew log?
5 days ago

Simon Harrer wrote:Also think about what you are doing when you quickly want to use a method or field and would like to know something about that. How often have you looked at the documentation, and how often have you really searched for the test cases? From our experience, documentation is, and will be, the first step in the search for info.


Perhaps so, but comments are often unclear, incomplete, unreliable, and possibly even obsolete. Comments often don't change along with the code. Unit tests, on the other hand, have to change, unless you want to leave them disabled or failing.

I coach teams to look at the unit tests first. When code is messy, tests usually get more attention than comments and are therefore relatively more reliable.
5 days ago

Jameson Locke wrote:My understanding of design patterns thus far is that it should not be implemented by force. Instead it should be used if your faced with a problem that suits the needs of a pattern (does that make sense?).


Absolutely. I'm glad to see that you were taught this at least. One thing though: rather than "a problem that suits the needs of a pattern" it really is "a problem that matches the context in which a pattern is suited" - Remember, a pattern that is applied to the wrong context often becomes an anti-pattern.

Here's what I can suggest: Try to get people to use your program and ask them what they might want to change to make it easier to use. Then look at your code and see how easy or difficult it would be to make those changes. If you find something that is difficult to implement because of the way the code is structured and not so much because of technological limitations, then you have found a potential context where some kind of pattern might apply.

Say for example one user wants to be able to use a Kinnect device as the input. Then maybe another user wants to use a keyboard instead of a mouse, and yet another would prefer a mouse rather than a keyboard. You have to ask yourself "How easy would it be to change this program so that it would support many different kinds of input devices?" Then see what kind of pattern you might use to easily solve this.
5 days ago
Hi Kamal,

Not all comments are bad or unnecessary although I have seen many that are. I think the authors explain the difference between good comments versus bad comments in the book. My rule of thumb is that if you can refactor the code to be so expressive as to make a comment redundant, then that's good. There are situations where it is useful to add comments that can help explain why something was done though. Comment that clarify the kind of things that can't be explained well through tests, such as why one algorithm was chosen over another, for example. It's all about creating context for the reader of the code, so they can get a good idea of what was going on in the programmer's mind when the code was written.
5 days ago