views:

339

answers:

6

I prefer coding standards to be logical. This is my argument for why the following set of standards are not.

I need to know one of two things: (1) why I'm wrong, or (2) how to convince my team to change them.


camelCase: Functions, class names, methods, and variables must be camelCase.

  • Makes it hard to differentiate between variables and classes
  • Goes against PHP's lowercase/underscored variables/functions and UpperCamelCase classes

Example:

$customerServiceBillingInstance = new customerServiceBillingInstance(); // theirs
$customer_service_billing_instance = new CustomerServiceBillingInstance();


Functions/methods must always return a value (and returned values must always be stored).

This appears on hundreds of our php pages:

$equipmentList = new equipmentList();
$success = $equipmentList->loadFromDatabase(true, '');
$success = $equipmentList->setCustomerList();
$success = $equipmentList->setServerList();
$success = $equipmentList->setObjectList();
$success = $equipmentList->setOwnerList();
$success = $equipmentList->setAccessList();

The return value is rarely used but always stored. It encourages the use of copy-and-paste.


No static methods

Lines like the following appear thousands of times in the codebase:

$equipmentList = new equipmentList();
$success = $equipmentList->loadFromDatabase();

I would prefer:

$equipmentList = equipmentList::load();

What reason is there to not use static methods or properties? Aren't static methods responsible for non-instance-specific logic? Like initializing or populating a new instance?


Your code is not OOP unless everything returns an object

There's a piece of code that performs a query, checks it several ways for errors, and then processes the resulting array. It is repeated (copied+pasted) several times, so I put it in the base class. Then I was told returning an array is not OOP.


How do you defend these practices? I really do need to know. I feel like I'm taking crazy pills.

If you can't defend them, how do you convince the adamant author they need to be changed?

+6  A: 

If you can't defend them, how do you convince the adamant author they need to be changed?

By giving strong/valid arguments! Still I think you should only change them when your arguments are really strong! Because most of the programmers at work are used to these coding standards which is a big point why to use them.

==

Like others told before this is pretty subjective, but these are mine opinions/arguments.

1. camelCase: Functions, class names, methods, and variables must be camelCase.

I would use the PHP style if I code in PHP and the Camelcase style if I code in Java. But it does not matter which style you choose as long as you stay consistent.

2. Functions/methods must always return a value (and returned values must always be stored).

This is nonsense in my opinion. In almost all programming languages you have some sort of 'void' type. But from a testing point of view most of the times it is useful if your function are side effect free. I don't agree that your production code should always use the return value especially if it does not have any use.

3. No static methods

I would advice you to read static methods are death to testability from misko

During the instantiation I wire the dependencies with mocks/friendlies which replace the real dependencies. With procedural programing there is nothing to “wire” since there are no objects, the code and data are separate.

Although PHP is a dynamic language so it is not really a big problem. Still the latest PHP does support typing so that I still think most of times static methods are bad.

4. Your code is not OOP unless everything returns an object

I believe(not 100% sure) a truly OOP language should do this(return an object), but I don't agree with this like a of like languages like for example Java(which I believe is not trully OOP). A lot of the times your methods should just return primitives like String/Int/Array/etc. When you are copying and pasting a lot of code it should be a sign that something is not totally right with your design. You should refactor it(but first have a tests(TDD) ready so that you don't break any code).

Alfred
1) I think style does matter in that it can affect productivity but I would agree in that consistency is nice. 2) Testing ability is a good point. 3) Thank you for the article. This does make sense; I guess I'll re-evaluate my use of statics. 4) A truly OOP language, perhaps; but one needs to draw the line somewhere in a language like PHP. We could go further and write `return new true();` but we need to draw the line somewhere, and I believe that line is at Utility.
Tim
@Tim 1) I don't believe CamelCase/underscore would really hurt productivity as long as you stay consistent. I guess a lot of employees of your job are now more used to camelcase. 4) Everything object I agree with you :P.
Alfred
@Alfred: having a single naming convention for everything may not hurt productivity; but if you have a distinct naming convention for each type of names (e.g. TitleCase for class, lowercase for locals, UPPERCASE for globals, camelCase for functions/methods, etc and use them consistently) then *this* will increase your productivity. There is a loss of potential productivity from having only a single naming convention for everything.
Lie Ryan
@Lie you might be a little bit right about that, but by mixing styles you will have to learn more.
Alfred
@Alfred: Yes, if you have 20 different conventions, they will be tedious to remember. The average person can remember 7 pieces of information at a time; I will take using 5 distinct convention to be a good limit.
Lie Ryan
+5  A: 

Many of these coding standards are very subjective, but some important things to consider are:

  1. Get a single set of code naming and style rules, and follow them. If you don't have already defined rules, make sure to get rules figured out. Then work at refactoring the code to follow them. This is an important step in order to make it easier for new developers to jump on board, and keep the coding consistent among developers.

  2. It takes time and effort to change the coding standards your company puts in place. Any change to the rules means that the code really has to be gone through again to update everything to be consistent with the new standards.

Keeping the above in mind, and looking more along the lines of specific PHP coding standards. The first thing to look at is if your company uses any sort of framework, look at the coding standards for that framework as you may want to stick with those in order to stay consistent across all the code. I have listed links to a couple of the popular PHP frameworks below:

  1. Zend Framework Naming Conventions and Zend Framework Code Style
  2. Pear Code Standards

My personal preference in regards to your specific coding styles:

1. camelCase: Functions, class names, methods, and variables must be camelCase

Class names should be Pascal Case (Upper Camel Case).

So in your example:

class CustomerServiceBillingInstance
{
// Your class code here
}

Variables and functions I generally feel should be camel case.

So either one of these, depending on your preference in terms of underscores:

$customerServiceBillingInstance = whatever;
$customer_service_billing_instance = whatever;

2. Functions/methods must always return a value (and returned values must always be stored).

This one seems like extra code and like you could end up using extra resources. If a function doesn't need to return anything, don't return anything. Likewise, if you do not care about what a function returns, don't store it. There is no point in using the extra memory to store something you are never going to look at.

An interesting thing you may want to try on this one, is running some benchmarks. See if it takes extra time to return something and store it even though you are not looking at it.

3. Your code is not OOP unless everything returns an object

In this instance, I feel you have to draw the line somewhere. Performance wise, it is faster for you to do:

return false;

Than to do:

return new Boolean(false); // This would use up unnecessary resources but not add very much to readability or maintainability in my opinion.

Defending a coding standard

To defend a coding standard that you feel is right (or showing why another is not as good), you really are going to have to bring up one of two points.

  1. Performance. If you can show that a particular coding standard is adversely affecting performance, you may want to consider switching.

  2. Maintainability/Readability. Your code should be easy to read/understand.

The goal is to find the happy median between performance and the maintainability/readability. Sometimes it is an easy decision because the most maintainable option is also the best performing, other times there is a harder choice to be made.

Brian
The problem is that there is already a pretty clearly-defined set of conventions, but I believe they are holding us back. I have a hard time obeying rules that are illogical in my eyes (the cases I posted in the question were just scratching the surface). I'm pretty sure the rules are the result of a single coder, and it was before my time.
Tim
Thanks for the edit--but how would I convince Him of this? My suggestions/complaints are all responded to with a "well it's all in the official doc!"
Tim
Perhaps another thing to consider is if you have more than two developers, but the team is still somewhat on the smaller side, hold a meeting and "review" the coding standards. Ideally it should go with what all the developers overall feel is the best.
Brian
We had a meeting and it was pretty much just re-affirming the doc... Arguing for performance, though, is a good idea. I think I need to explain things in terms of money and time before they will listen.
Tim
+1 for Zend conventions
Alex
+1  A: 

Standards and conventions exist for many reasons, but most of these reasons boil down to "they make code easier to write and maintain." Instead of asking "is this the correct way to do X?" just cut right to the chase and ask if this criterion is met. The last point in particular is simply a matter of definition. OOP is a means, not an end.

Seth
But what if the person you are asking is OCD and you don't want to turn around 5 times and tap a doorknob twice before "doing X"?
Tim
Then that person can deal with it, or handle the standards issues themselves.
SimpleCoder
+1 for "OOP is a means, not an end". Too many people act like it's gospel -- it is not.
Billy ONeal
+5  A: 

I would suggest trying to list down the goals of your coding standards, and weigh them down depending on which goals is the most important and which goals are less important.

PS: I don't speak PHP, so some of these arguments may contain blatantly incorrect PHP code.

camelCase: Functions, class names, methods, and variables must be camelCase.

Workplace's Apparent Reason: "consistency" at the cost of "information density in name".

Argument:

1) Since 'new' keyword should always be followed by a class, then you can easily spot illegal instantiation, e.g.:

$value = new functionName();
$value = new local_variable();
$value = new GLOBAL_VARIABLE();

would raise an alarm, because 'new' should be followed by a TitleCase name.

$value = new MyClass(); // correct

Relying on Case makes it easy to spot these errors.

3) Only functions can be called, you can never call variables. By relying on Case Rule, then we can easily spot fishy function calls like:

$value = $inst->ClassName();
$value = $inst->instance_variable();
$value = $GLOBAL_VARIABLE(); 

3) Assigning to a function name and global variables is a huge deal, since they often lead to behavior that is difficult to follow. That's why any statement that looks like:

$GLOBAL = $value;
$someFunction = $anotherFunction;

should be heavily scrutinized. Using Case Rule, it is easy to spot these potential problem lines.

While the exact Case Rule may vary, it is a good idea to have different Case Rule for each different type of names.

Functions/methods must always return a value (and returned values must always be stored).

Workplace's Apparent Reason: apparently another rule born out of blind consistency. The advantage is that every line of code that isn't a flow control (e.g. looping, conditionals) is an assignment.

Argument:

1) Mandatory assignment makes unnecessary long lines, which harms readability since it increases the amount of irrelevant information on screen.

2) Code is slightly slower as every function call will involve two unnecessary operation: value return and assignment.

Better Convention:

Learn from functional programming paradigm. Make a distinction between "subroutine" and "functions". A subroutine does all of its works by side-effect and does not return a value, and therefore its return value never need to be stored anywhere (subroutine should not return error code; use exception if it is really necessary). A function must not have any side-effect, and therefore its return value must be used immediately (either for further calculations or stored somewhere). By having side-effect free function policy, it is a waste of processor cycle to call a function and ignoring its return value; and the line can therefore be removed.

So, there is three type of correct calls:

mySubroutine(arg);            // subroutine, no need to check return value
$v = myFunction(arg);         // return value is stored
if (myFunction(arg)) { ... }  // return value used immediately

you should never have, for example:

$v = mySubroutine(arg);  // subroutine should never have a return value!
myFunction(arg);         // there is no point calling side-effect free function and ignoring its return value

and they should raise warning. You can even create a naming rule to differentiate between subroutine and functions to make it even easier to spot these errors.

Specifically disallow having a "functiroutine" monster that have both a side-effect and return value.

No static methods

Workplace Apparent Reason: probably someone read somewhere that static is evil, and followed blindly without really doing any critical evaluation of its advantages and disadvantages

Better Convention:

Static methods should be stateless (no modifying global state). Static methods should be a function, not subroutine since it is easier to test a side-effect-free function than to test the side-effects of a subroutine. Static method should be small (~4 lines max) and should be self-contained (i.e. should not call too many other static methods too deeply). Most static methods should live in the Utility class; notable exceptions to this is Class Factories. Exceptions to this convention is allowed, but should be heavily scrutinized beforehand.

Your code is not OOP unless everything returns an object

Workplace Apparent Reason: flawed understanding of what is OOP.

Argument:

Fundamental datatypes is conceptually also an object even if a language's fundamental datatype doesn't inherit from their Object class.

Lie Ryan
An objective and detailed analysis! Thanks!
Tim
A: 

The one aspect of this that I don't see anybody else commenting on is the "2. Functions/methods must always return a value (and returned values must always be stored)."

If you're not using exceptions, then any function that can fail does have to return a value. The last clause is misworded; return values don't always need to be STORED, but they do always need to be CHECKED. Again, that's IF you're not using exceptions, which isn't common these days, but it's still worth mentioning.

mjfgates
If you really did mean "misworded", I assure you, it's not... I think "misguided" is more accurate. :)
Tim
+2  A: 

Many of those are matters of taste. You can argue for or against camelCase all day.

However, the storing of return values that are never used is Wrong. There is no value to the code. The code is not executed. You might as well sprinkle $foo = 47 * 3; around the code.

Any code that is not doing something useful must be removed.

The bigger issue is, if you're working for a clueless manager, it may be time to move.

Andy Lester
Actually, I think the camelCase argument is more than just taste (see Lie Ryan's detailed analysis above). My manager is not clueless but doesn't personally take part in the code itself. Most of this stuff comes from the "senior" programmer ("senior" not in age or skill, but rather time at this company). I do at times want to move, but feel I haven't been there long enough.
Tim
How much misery do you think you have to put in before you can leave?
Andy Lester
It's been 4 months... I think I could survive another 8 :) This is my first "corporate" job in the programming world and I am concerned about future employers seeing a particularly short stint at my previous job.
Tim
Sure, I'll agree. If it's your first job, definitely stick it out. The coding standards may suck, but there are plenty of things you can still learn. Your job right now is to soak up as much knowledge of the working world as you can. I'll also refer you to my blog http://theworkinggeek.com about stuff like this.
Andy Lester