Array to and from Mapping conversion

[uda]

When building RESTful or RESTlike apps, I came across the request to allow passing bundles of information in the following format:

[
  {
    "field": "fname",
    "value": "John"
  },
  {
    "field": "lname",
    "value": "Doe"
  }
]

For storing purposes, I needed it in the following format:

<?php
array(
  'fname' => 'John',
  'lname' => 'Doe'
);

So utilizing the Arr class I added methods to convert back and forth between these two formats.

[rjd22]

@uda thank you for your PR! :). I think this is really REST specific and most likely does not belong in the Arr class. Are json collections always formatted like that? If so I would put it in a new Json class. If not I would even think about making a Rest class.

@shadowhand can you give your 2c?

[shadowhand]

I'm not opposed to the functions. The placement does seem to be a bit odd, though I can't think of a better place for it.

[rjd22]

@shadowhand maybe make a Rest class for them? They seems to be only usefull for rest operations.

[shadowhand]

@rjd22 the classes aren't named by purpose though. The Date class does things with dates, the Cookie class does things with cookies, and so Arr class does things with arrays. "REST" is not a type, but a pattern.

[rjd22]

@shadowhand Yea you're right. Maybe we could do something with the term collection?

Something like convertColumnToFlat and convertFlatToColumn. Like in this function: http://php.net/manual/en/function.array-column.php

[shadowhand]

I'm not entirely sure this is about "columns" either, since the goal is converting from a "schema" to "schema-less" or "generic" mapping. Perhaps it could be toMapping and fromMapping?

[rjd22]

@shadowhand I'm okay with that :) @uda would you be able to change the methods to those names?

[uda]

Yes @rjd22, i will do it soon

[uda]

I fixed array backwards compatibility issue

[rjd22]

@uda I'm willing to merge this but can you maybe add some simple tests? That way to know how it needs to be used.

[rjd22]

@uda I'm merging this. I will make a ticket that it will need tests :)

[acoulton]

@rjd22 not really any of my business, but merging this without tests feels like a mistake. Ohanzee will just end up like Kohana, with a bunch of untested code that can't easily be refactored and a big wishlist somewhere of the tests nobody's got round to writing yet.

[shadowhand]

I agree.

[rjd22]

@acoulton @shadowhand While I most of the time agree with that it needs test we should also be careful not to discourage other developers from contributing.

This is mostly isolated code and tests can always be added later but keeping good code out to wait for tests will end up with no contributions at all.

It's better practise to motivate people by merging the PR for the hard work they did and ask them if they could add some tests. And if the don't want to do that I will make some myself.

[acoulton]

@rjd22 afraid I disagree. Most of the major projects I've used and occasionally contributed to in the past have had a "no tests, no merge" policy. It doesn't seem a barrier to them building a community and getting contributions.

You're right that it's important not to discourage people - responding politely, offering to help with tests, even writing some and sharing them so they can add them to their PR are all good things and most importantly will help them to grow as contributors. I've had the benefit of help like that from other projects in the past, and I've really valued it. When a project that doesn't take any old thing helps you to improve your contribution, it's a good feeling.

Merging their PR without tests and hoping that at some point in the future disconnected from them you might get round to writing some yourself doesn't help that contributor.

What it does do is reduce the overall quality of the package - no matter how isolated or "good" the code - and as a result makes it less likely that people will use it so the pool of potential contributors shrinks. Even for people who do use it, the absence of comprehensive tests for existing code may make them less likely to propose changes.

At my work, we exclusively use a BDD process to build our own code. When looking for an external package, the quality of the test suite is an important consideration. My sense is that there's an increasing number of places like that.

Just my £0.02

[rjd22]

@acoulton This would only work when you already have a big pool of contributors. Ohanzee has a really small pool of those and not having tests does not mean the code is "bad".

It already has been reviewed by multiple people and checked op potential errors. And it already has been hand tested. I agree that we should not just merge everything but we should also be more considerate to our contributors that already did good work and not push everyone into our "ideal" world.

Like I said I will add some tests soon but I still believe that bit more flexibility will give projects like ohanzee and kohana for that matter more freedom to grow.

[shadowhand]

I agree with @acoulton completely. When contributors refuse to write tests, it shifts the burden of testing onto the maintainers and defers finding bugs that would otherwise be missed during review and "hand testing". As the gatekeepers, we have a responsibility to ensure that code coming in meets a standard. And I strongly feel that one of those standards should be "fully tested".

[rjd22]

@shadowhand Okay, lets agree to have different points of views. But I will make tests later today for these functions.

On a different note I wonder why the tests are not working properly right now. Also there are tests that test functions that are not there like Arr::range.

[uda]

@rjd22 @shadowhand @acoulton, in some freakish way, I agree with all... I agree that sometimes the requirement of tests puts some complications for contributors who are not used to developing with tests (by them selves or in general, sadly enough). On the other hand, I agree that pushing code to a public project w/o tests is bad behavior. really bad, and I apologize for that, the reasons don't matter. If @rjd22 won't make tests, I'll convert ours to match Ohanzee's conventions.

[uda]

I added tests, creating PR now.