[cfoley]

That Point class is a good example of poor design in the Java API. They provided an int version of setLocation. int getters for X and Y would have been better than exposing the variables.

[xclite]

cfoley, on 04 February 2011 - 06:23 AM, said:

I was sure I posted here this morning.

Anyway, the gist of what I said was that that it's a bad idea to write code that makes thing easier in some way for the computer (i.e. uses less memory or runs quicker). Most of the time you can end up with bloated, buggy, unmaintainable code for negligible gain. Focus on writing clean, clear code and save optimisation for when you have a real performance problem.

I missed this earlier, but it's a good point and to quote Knuth:

Quote

Premature optimization is the root of all evil (or at least most of it) in programming.

At least I've seen it attributed to Knuth.

cfoley, on 04 February 2011 - 11:33 AM, said:

That Point class is a good example of poor design in the Java API. They provided an int version of setLocation. int getters for X and Y would have been better than exposing the variables.

They have the getters and the setters. There is nothing dangerous or poor about how you interact with x and y by calling location.x and location.y.

[Zekorov]

I've been working on a program for like the past whole semester.... and frankly it still sucks but i am making progress with it. So far, from the experiences i have had with it, a programmer does not want to make things too confusing. Do not just start writing code unless you can write it exceptionally well on practiced values. You have to plan code out first and think about what you're writing so that it will make sense in the long run to you, and other programmers that see your code too. When it makes sense, the code is easier to handle and will give much less of a headache in the end. Also, a programmer does not want to make objects for everything. object oriented programming is great and works well if you see what needs to be an object and what doesn't. For example, when i first started, I tried making my whole battle sequence in my little text game an object. That is something that should not be an object, especially if you want the program to "remember" statistical values such as health, experience, strength, defense, etc because if it is an object inside a loop, it will restart everything every time you go into battle as it recreates the object over and over. Another do not is making things that are separate from one another that need to be somehow conjoined. I have learned this the hard way. I made my user stats class separate from the monster stats class and it created a million problems in the long run when i tried creating multiple monsters to battle. Instead, i used inheritance from a MainStats class and now the two subclasses intermingle quite well when i need them too. But yeah, just some thoughts.

[xclite]

Another example of using public vars is desigining a node class for a data structure. You're using an inner class - the node is NEVER exposed to people using your structure. I'll be damned if, in my LinkedList, I'm going to call node.getNext().setNext(node.getNext().getNext().getPrev()). How about I just have public links to next and prev? suddenly it's more readable, has less noise, and makes it easier to follow.

Note: I am aware that the line I made up isn't a semantically correct line used to rearrange the list.

This post has been edited by xclite: 04 February 2011 - 09:44 AM

[cfoley]

By exposing the variables, they have set their implementation in concrete for all time. This is made worse by it being an API class and not an application class. If they wanted to make the Point class thread safe at some future point they could not. You can't synchronize variables and you can't make an API's variables private for risk of breaking every application that was ever written in the language.

Furthermore, if I decide I need a thread safe point class, I cannot simply extend the one you linked to. It's public members become part of my public interface. I can override the methods to synchronize them but I cannot do that with the variables. I'd have to make a wrapper class.

I think I know why they might have done it. They have:

public double getX()

If you add

public int getX()

it won't compile. What they should have done is omething like this:

public int getXasInt()

Quote

Another example of using public vars is desigining a node class for a data structure. You're using an inner class - the node is NEVER exposed to people using your structure. I'll be damned if, in my LinkedList, I'm going to call node.getNext().setNext(node.getNext().getNext().getPrev()). How about I just have public links to next and prev? suddenly it's more readable, has less noise, and makes it easier to follow.

You can make it package access and still access it from the enclosing class. Damn, make it private and access it from synthetic accessor methods. In this case, the Node class is an implementation detail of the data structure. Making it public here is probably not the sin it is in top level classes but it's still confusing to the programmer who thinks it will be accessible from anywhere. Leave it default access and make the intent clear.

[xclite]

While the point about thread safety is a good one (although anything accessing the shared point should be using the same mutex, so the point itself doesn't necessarily need synchronization [not that I think workarounds should be required]), we've noted that not every data member has to be declared private or protected with a setter or getter, which is my point. Doing so isn't something that should be expressly forbidden, but examined in unique cases.

[SpeedisaVirus]

In most cases which they are not private or protected they should be. The circumstances that a non final member should be public is like finding a three armed man. Not all that common.

[xclite]

The fact that you can't say "never happens" means nobody should be saying "never do it."

[baavgai]

cfoley, on 04 February 2011 - 10:43 AM, said:

Also static methods are pretty useful - I would hate to have to create a Double object to call parseDouble() on a string, or pretty much the entire Math library.

A creator pattern is nice. Though it could just as easily support public Double(String s). Hardly required or a compelling reason for static.

xclite, on 04 February 2011 - 11:43 AM, said:

You're using an inner class - the node is NEVER exposed to people using your structure. I'll be damned if, in my LinkedList...

Excellent example. I agree. Some classes are objects with behavioral patterns than need to be encapsulated. However, some are just complex data types, holding information and little more. Such things like private classes needn't worry about data wrapping.

OO design techniques address scope of use. I want to limit the impact of changes and the rest of the program from undefined behavior. A class whose only job is to be a data container can be completely open, depending on the scope of the class and structure of the program.

[xclite]

baavgai, on 04 February 2011 - 12:15 PM, said:

cfoley, on 04 February 2011 - 10:43 AM, said:

Also static methods are pretty useful - I would hate to have to create a Double object to call parseDouble() on a string, or pretty much the entire Math library.

A creator pattern is nice. Though it could just as easily support public Double(String s). Hardly required or a compelling reason for static.

I think you accidentally quoted cfoley =p

I could live with that, although I still stand in support of utility methods such as the Math library. I've also written a custom library that added ways to manipulate strings - certainly not methods that relate to any kind of instantiated object (and you can't change/extend the String class in Java), so they made sense to be static.

This post has been edited by xclite: 04 February 2011 - 10:23 AM

[japanir]

I'd like to comment about the public data members.
set\get methods also allow you to check bad values passed to your variables.
Perhaps you have a variable that should hold only even integer values (I know it's unlikely, but that's just an example)
if that value is declared as public, you can actually assign it any value. However, if you declare it as private, and create a set method, you can add code to check the value passed to that variable and throw exception, or maybe change the value if an odd value is passed.
And if tommorow your manager says that he decided that the variable should hold only numbers divisable by 3, all you have to do is a small change to your set method.
However, If you used a public variable, you'll have to make much more changes to your code.
set\get methods offer a lot of modularity to the code.

[SpeedisaVirus]

xclite, on 04 February 2011 - 05:13 PM, said:

The fact that you can't say "never happens" means nobody should be saying "never do it."

When is the last time you have seen someone with 3 arms.

[macosxnerd101]

Expanding on japanir's point, it also allows for Polymorphism if you need to override the get/set methods in the subclasses. You can't add restrictions with public variables. You can with methods.

[xclite]

SpeedisaVirus, on 04 February 2011 - 01:14 PM, said:

xclite, on 04 February 2011 - 05:13 PM, said:

The fact that you can't say "never happens" means nobody should be saying "never do it."

When is the last time you have seen someone with 3 arms.

More times than my code has been complained about =p

You all are making great points for using getters and setters - and I agree that they are a good idea. I feel like my opposition to the words always and never is being taken as "getters/setters are useless."

[Dogstopper]

With few exceptions, never make an entire GUI through an object of that class. Somewhere in there, you should override some form of component. Otherwise, everything gets mushed together and makes it difficult to read.

---

Also, never leave methods with more than 50 or so lines be. If you have 50 lines in a method, you are likely approaching it the wrong way. Take a step back and ask yourself how you can make it more OO or functional.