Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transform back #10

Closed
rogierborst opened this issue Dec 19, 2013 · 15 comments

Comments

Projects
None yet
5 participants
@rogierborst
Copy link

commented Dec 19, 2013

I was wondering, isn't this a very one-way transformation?
Let's say my repo is using snake_case and with fractal I'm outputting as camelCase.

Now the client wants to create a new instance and for all he knows, we're using camelCase throughout our api. So he'll post with camelCase definitions.

Does fractal then provide a way to transform back to snake_case?

@philsturgeon

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

I think talking about converting snake_case to camelCase might be confusing the issue a little, but we can have this discussion with a slightly more common use-case.

Somebody asked me if Fractal would be good for converting their JSON into a format for their models to save. For me this was not something I ever considered doing for Fractal as the entire point for me was just to convert disparate and potentially complex data sources into consistent output for easy formatting and output.

Let's try out some theory code. It would be possible for me to have an input / output approach, where output is the current transform method and input takes data in whatever format and converts it to an instance of whatever it is you are trying to make.

<?php namespace App\Transformer;

use League\Fractal\TransformerAbstract;
use Opp;

class OppTransformer extends TransformerAbstract
{
    /**
     * Available embeds
     *
     * @var array
     */
    protected $availableEmbeds = [
        'category',
        'merchant',
    ];

    /**
     * Turn this item object into a generic array
     *
     * @param array $input
     *
     * @return Opp
     */
    public function input(array $input)
    {
        $opp = new Opp([
            'id'             => (int) $input['id'],
            'type'           => (string) $input['type'],
            'name'           => (string) $input['name'],
            'teaser'         => (string) $input['teaser'],
            'details'        => (string) $input['details'],
            'trending'       => (bool) $input['trending'],
        ]);

        $opp->ormMagicRelationship($input['category_id']);
        $opp->ormMagicRelationship($input['merchant_id']);
    }

    /**
     * Turn this item object into a generic array
     *
     * @return array
     */
    public function output(Opp $opp)
    {
        return [
            'id'             => (int) $opp->id,
            'type'           => (string) $opp->type,
            'name'           => (string) $opp->name,
            'teaser'         => (string) $opp->teaser,
            'details'        => (string) $opp->details,
            'trending'       => (bool) $opp->trending,
        ];
    }

    /**
     * Embed Category
     *
     * @param Opp $opp
     *
     * @return League\Fractal\Resource\Item
     */
    public function embedCategory(Opp $opp)
    {
        return $this->item($opp->category, new CategoryTransformer);
    }

    /**
     * Embed Merchant
     *
     * @param Opp $opp
     *
     * @return League\Fractal\Resource\Item
     */
    public function embedMerchant(Opp $opp)
    {
        return $this->item($opp->merchant, new MerchantTransformer);
    }
}

Is that the sort of thing you want? The embed logic would need to stay as output only, and you could make other instances and whatnot. You could hide more of the logic in your models/repositories/other domain code, and just use this as a general wrapper for things, but I have to wonder if this is even the responsibility of Fractal.

If Fractal handles output, nesting/embedding of relationships, etc, shouldn't your input be handled in a model/repository somewhere? There is no complicated output, nesting, etc for saving some shit. It's just passing data to some methods.

@alexbilbie

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

I like @philsturgeon's suggestion, reminds me of __sleep() and __week()

@philsturgeon

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

@alexbilbie what about my last two paragraphs, where I say "yeah but fuck it"?

@fideloper

This comment has been minimized.

Copy link

commented Dec 19, 2013

Does handling input result in creating (essentially) a data-transfer object that would then be consumed or tranformed into, say, a Laravel Model (for example)?

If so, I'm not sure I see the value.

Edit: Knowing that Opp() is domain-level code for the app the above class might appear in, the Transformer now makes more sense. That's not really a data-xer object, but we're looking at the actual transformer of input to an business logic entity.

I think in any case, I still would be on the same page of "this isn't the responsibility of Fractal". Perhaps a separate package for handling input.

I think it would be most useful when doing DDD type code instead of model (such as Eloquent) driven. With ORMs, you can (generally) populate a model with the input, skipping the need for a transformer.

The biggest point I'd make is that it confuses the purpose of Fractal a bit. Definitely, in either case, make it a separate package.

@alexbilbie

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

Sorry I somehow ignored the last two paragraphs.

Fractal is an output library so it isn't it's responsibility, if you're expecting snake_case to be POSTed back to your API then that should be dictated in your API documentation

@rogierborst

This comment has been minimized.

Copy link
Author

commented Dec 19, 2013

@alexbilbie That's not a very HATEOAS thing to do, depending on API documentation.

As for whether or not it's Fractals job, maybe not. But I can see myself creating the opposite of a Fractal transformer in my repository, writing yet another array with (to refer back to my original example) camelCase to snake_case transformations. I was just wondering if there wouldn't be a more efficient way to go about that.

@rogierborst

This comment has been minimized.

Copy link
Author

commented Dec 19, 2013

@philsturgeon Btw, in your example code inside the input method, I guess you forgot to change $opp into $input.

I see you are repeating the same structure too, maybe that's just the only way to go about it. In that case, yeah, forget about it, it's probably not Fractals responsibility.
In my current project (where I'm not using Fractal yet, but a personal transformer) I'm about to start working with input, so I haven't yet given it much thought. Will check back if I can think of something clever :)

@philsturgeon

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

@roggel you read the initial email, not the actual issue example which was updated earlier. :)

@philsturgeon

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

@roggel output needs a transformer (lets be honest, its a callback to convert some random data to an array) but it also needs:

  • Embedded Relationships
  • Meta Data
  • Links
  • "Smart Syntax" to limit, order, etc
  • Pagination

Fractal supports (or will support) all of the above features.

Input requires:

  • Calling some methods to save stuff

After that, you output the result, using Fractal. Your input method could be ANYWHERE else, and has no relation to anything else that fractal currently does or will do.

@rogierborst

This comment has been minimized.

Copy link
Author

commented Dec 19, 2013

Okay, that's a really clear explanation. There is absolutely no need for Fractal to support this.

Now, when I get to saving input in my current project, it's good to know I don't have to come up with something clever. Thanks!

@alexbilbie

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

@roggel regarding HATEOAS I didn't consider it in my response because I largely disagree with it's principles, don't be a dick and make me work out how your API works, just document it

@rogierborst

This comment has been minimized.

Copy link
Author

commented Dec 19, 2013

One thing doesn't have to exclude another. I'd prefer to do both, provide an api that documents itself and provide separate, extensive documentation.

@awright022

This comment has been minimized.

Copy link

commented Dec 19, 2013

@philsturgeon I think "Input requires: Calling some methods to save stuff" Might be a bit of an oversimplification. You could come up with a large list of needs for Input which might be abstracted from "just save some values." Validation and creation of nested/related data etc. But I do think it's outside of fractal's scope, especially since these other things are often handled at different layers.

@philsturgeon

This comment has been minimized.

Copy link
Member

commented Dec 19, 2013

@awright022 I stand by my comments. Validation is sometimes just if ($foo > 3) but has a million packages for it already. So use those for that, or ifs, then carry on with "save stuff". It's still not something fractal needs to care about.

As for nested/related data, that is not a concern for input. You're more likely to post a "category_id" than post an entire { category : { id : 5, name : "Cheese", icon : "cheese.gif" } } so the complications of nested data remain purely as an exercise for the output.

@philsturgeon

This comment has been minimized.

Copy link
Member

commented May 10, 2018

Relevant update on #65 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.