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

Break object initializers if there are 3 or more properties #446

Closed
belav opened this issue Sep 29, 2021 · 4 comments · Fixed by #517
Closed

Break object initializers if there are 3 or more properties #446

belav opened this issue Sep 29, 2021 · 4 comments · Fixed by #517

Comments

@belav
Copy link
Owner

belav commented Sep 29, 2021

var myObject = new MyObject { Property1 = true, Property2 = false };

Should we always break if there are two properties? Should be easy to test out across all the forked repos.

var myObject = new MyObject
{
    Property1 = true,
    Property2 = false
};
@belav belav self-assigned this Oct 10, 2021
@belav
Copy link
Owner Author

belav commented Oct 10, 2021

I am not sure if we really want this change. It leads to cases like this.

    var myList = new MyList<Point>
    {
        new Point
        {
            x = 10,
            Y = 10
        },
        new Point
        {
            x = 20,
            Y = 20
        }
    };
// vs
    var myList = new MyList<Point>
    {
        new Point { x = 10, Y = 10 },
        new Point { x = 20, Y = 20 }
    };

And

    list.Add(
        new RadioItem
        {
            Text = "text-foo",
            Value = "foo"
        }
    );
    list.Add(
        new RadioItem
        {
            Text = "text-bar",
            Value = "bar"
        }
    );
    list.Add(
        new RadioItem
        {
            Text = "text-baz",
            Value = "baz"
        }
    );
// vs
    list.Add(new RadioItem { Text = "text-foo", Value = "foo" });
    list.Add(new RadioItem { Text = "text-bar", Value = "bar" });
    list.Add(new RadioItem { Text = "text-baz", Value = "baz" });

Which I prefer with the condensed format.

But something nested like this I think is better expanded

    MyClass myClass = new MyClass
    {
        Value = "Foo",
        Thing = new MyThing { Number = 456, }
    };
// vs
    MyClass myClass = new MyClass { Value = "Foo", Thing = new MyThing { Number = 456, } };

A ton of real world examples are here

The official microsoft documentation has them expanded.

It might be worth trying to analyze csharpier-repos to get an idea if a large percent of the real world code had them expanded.

Also worth noting that prettier has been unable to find a good heuristic on when to expand objects

@belav
Copy link
Owner Author

belav commented Nov 21, 2021

The changes from #488 are related.

@loraderon
Copy link
Contributor

How about 3 or more?

With file scoped namespaces, a lot of smaller props will fit in 100 characters.

@belav
Copy link
Owner Author

belav commented Dec 20, 2021

This is the result of breaking things with 3 or more. https://github.com/belav/csharpier-repos/pull/17/files
I think I am on board with it.
My not very accurate scan of existing code from the repositories there - automapper favored keeping initializers on a single line. Most others favored breaking them.

@belav belav changed the title Consider always breaking object initializers if there are 2 or more properties Break object initializers if there are 3 or more properties Dec 20, 2021
belav added a commit that referenced this issue Dec 20, 2021
belav added a commit that referenced this issue Dec 20, 2021
* try out break at 3

* Breaking initializers if there are 3 or more

closes #446
@belav belav added this to the 0.13.0 milestone Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants