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

The use of underscores #116

Open
bbatsov opened this issue Feb 22, 2015 · 39 comments
Open

The use of underscores #116

bbatsov opened this issue Feb 22, 2015 · 39 comments

Comments

@bbatsov
Copy link
Contributor

bbatsov commented Feb 22, 2015

I think the heavy use of an underscore prefix in Volt is problematic as the widely accepted use for it in the Ruby community is for unused variables/params. Every time I see something like page._todos I'm taken aback and I can imagine there will be cases when someone would wonder if _something is something unused or something from Volt. What's the rationale behind this naming scheme?

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

Ryan will be able to give you a better answer, but using an underscore methods on a Model is just a way to set/get the data without the need to create an accessor (so is handy for being able to quickly build a prototype).

I suggested suffixing the underscore, but can live with the prefix. Can you think of any alternatives?

@andrew-carroll
Copy link

I'm not a fan of the underscore either. while the accessor is incredibly
helpful, the resulting code is difficult to read and unintuitive. While it
feels cool as a developer to type it, reading a bunch of these in a row can
easily get confusing. If I want to reference page._items, I can't have
another page_items without having to double take.

I suggest a minimal change to page.v_items. It's only a character longer
but conveys intent much more cleanly.
On Feb 22, 2015 4:39 AM, "AstonJ" notifications@github.com wrote:

Ryan will be able to give you a better answer, but using an underscore
methods on a Model is just a way to set/get the data without the need to
create an accessor (so is handy for being able to quickly build a
prototype).

I suggested suffixing the underscore, but can live with the prefix. Can
you think of any alternatives?


Reply to this email directly or view it on GitHub
#116 (comment).

@bbatsov
Copy link
Contributor Author

bbatsov commented Feb 22, 2015

Ryan will be able to give you a better answer, but using an underscore methods on a Model is just a way to set/get the data without the need to create an accessor (so is handy for being able to quickly build a prototype).

Got it - I'll assume we just need some marker for method_missing. Where's the relevant code, btw?

I suggested suffixing the underscore, but can live with the prefix. Can you think of any alternatives?

Haven't really thought about this - v_something seems like a simple enough solution. Some easy to type unicode character might be another reasonable solution. Or we can use [:attr_name] on models?

@dfyx
Copy link
Contributor

dfyx commented Feb 22, 2015

Some easy to type unicode character might be another reasonable solution.

I don't really like that idea. There are hardly any unicode characters that are easy to type on all common keyboard layouts.

@bbatsov
Copy link
Contributor Author

bbatsov commented Feb 22, 2015

It's not my favourite idea, either. :-)

Thinking a bit more I'd definitely would prefer something like:

page[:name] = x
page[:name]
# and maybe aliases ala Rails's get_attribute/set_attributed, but way shorter
page.set(:name)
page.get(:name)

This also makes it clearer that you're just setting/getting something without any magic. Clarity is important.

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

@andrew-carroll & @dfyx - interesting ideas. What about going a little further and maybe using a small word, such as var_something or tmp_something - that way they can easily be searched for once prototyping has been completed :)

@bbatsov Sorry I wasn't clear in my first post, basically the underscore is for prototyping only, once you know what you want, you add the accessors to the model classes and remove the underscore, e.g.:

class Item < Model
  field :name, String
end

then you could say:

item.name
item.name = 'Ryan'

@bbatsov
Copy link
Contributor Author

bbatsov commented Feb 22, 2015

@AstonJ Yeah, I gathered as much already. But I also assume that those prototype methods are trivial accessors (set/get something), so mapping them to [] makes sense in my mind. Sure, this way you'll have to type a bit more when you "finalize" the attributes, but this can always be automated in a good editor.

@andrew-carroll & @dfyx - interesting ideas. What about going a little further and maybe using a small word, such as var_something or tmp_something - that way they can easily be searched for once prototyping has been completed :)

A longer prefix kind of defeats the purpose of this feature IMO. On a related note - a prefix like var_ or tmp_ will leave a lot of room for alternative interpretations. Searching for .v_something is unlikely to yield a lot of false positives, btw.

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

@bbatsov I suggested var_something purely for aesthetics and code readability - is it nicer than a page full of .v_somethings for instance? I haven't checked yet but I would create a page with each of the variations to see what feels nicest :)

I'm unsure about mapping to [] but will look forward to hearing what Ryan and everyone else thinks.

@danielpclark
Copy link

I haven't been in the practice of ever using underscore methods. It doesn't bother me one way or the other. If anything you might create symmetry by adding a tailing underscore to the object on which the method is called. page_._todos This way you'll know more intuitively that all methods ending with underscore have methods that should be prefixed with underscore simply by the visual cue. Just an idea. Again I'm fine with the current syntax.

I will admit the first time I saw page._todos it looked a bit alien to me, but that wasn't a problem.

@ryanstout
Copy link
Member

Hey everyone, so a little background on why the _'s are there. As has been mentioned, once your out of the prototype stage, you can disable them and use field :name in the models, then do model.name = '...' directly.

We can't use something like .name and .name= directly because then errors get swallowed when you call a method that doesn't exist. Having a different syntax makes it explicit that you are accessing a property that may or may not be defined. And prevents typos from being swallowed for normal methods.

I thought about using model[:name] = '...', but one thing I wanted is to be able to maintain "universal access" with _ accessors. So if I have something like this:

class Person < Volt::Model
end

I can do something like this:

person = Person.new
person._name = 'Ryan Stout'

Now say I want to change the way I store name, but keep the same interface:

class Person < Volt::Model
  def name=(name)
    self._first_name, self._last_name = name.split(' ')
  end

  def name
    _first_name + ' ' + _last_name
  end
end

The way this works currently is that I can now do:

person = Person.new
person._name = 'Ryan Stout'

and it will call the #name= method. Obviously I could do this using person[:name] = '..', but I feel like people don't expect this to actually call a method. That said, it is a bit more rubyesque, and I'm coming around to changing it. Feel free to argue here for using []. My major concerns with [] are:

  1. It doesn't look like a method call
  2. It's a little harder to track down when you do add the field in and want to change them.
  3. I think a bunch of nested [: ..]'s can be harder to read.

Ok, feel free to argue here :-)

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

I'm not convinced by [: ] either tbh. I'm growing towards var_something though for a few reasons:

  • it looks natural
  • it isn't jarring
  • you can quickly see it's related to prototyping
  • it will probably look nicest on a real life page
  • switching to it from underscore will be trivial enough

What are your own thoughts about v_something var_something Ryan?

@bbatsov
Copy link
Contributor Author

bbatsov commented Feb 22, 2015

it looks natural

It's anything but natural. Overloading existing semantics on a per-project basis is rarely a good idea. One can get used to this, but should they?

you can quickly see it's related to prototyping

That's not a property of using _ per se, but of using some convention.

it will probably look nicest on a real life page

That's super subjective.

We can't use something like .name and .name= directly because then errors get swallowed when you call a method that doesn't exist. Having a different syntax makes it explicit that you are accessing a property that may or may not be defined. And prevents typos from being swallowed for normal methods.

Not necessarily - we can log method_missing calls on model objects. I agree to some extend about the syntax bit, but I profoundly dislike the use of the prefix because of code like:

<input type="checkbox" checked="{{ _checked }}" />

A newcomer might interpret this in at least 3 ways... If something is ambiguous then it can likely be improved upon.

and it will call the #name= method. Obviously I could do this using person[:name] = '..', but I feel like people don't expect this to actually call a method. That said, it is a bit more rubyesque, and I'm coming around to changing it. Feel free to argue here for using []. My major concerns with [] are:

  1. It doesn't look like a method call

object.x = y doesn't look like a method call either. I'd say this is a pretty subjective point.

  1. It's a little harder to track down when you do add the field in and want to change them.

Perhaps.

  1. I think a bunch of nested [: ..]'s can be harder to read.

Seems unlikely that a lot such code will be produced in practice.

One of the biggest issue of Rails is the overuse of metaprogramming for things that can be done in a simpler manner. In my book explicit beats implicit most days of the week and the value of many of the shortcuts that implicit solutions provide is undermined by their maintenance overhead. What's so hard about adding a few fields explicitly? How much time do we actually save in the prototyping mode and at which cost?

I admit I haven't read all the docs, but the first few chapters use prototype attributes quite a lot without a single line of explanation regarding them.

I find it hard to believe we can't find a better solution to the problem at hand (even if it's just a better naming convention that would clearly separate those attributes from unused locals).

@ryanstout
Copy link
Member

@bbatsov so on the swallowing of errors, in order to use a property before its been assigned, it has to return something when you ask for it, in our case we return nil. If it just threw an error or logged one, then you couldn't do bindings without initializing them first. That would add a lot more code.

So if your just coming from rails, you might not fully understand why you can't just add fields explicitly all of the time. The main reason is that most fields get bound to properties on the controller. You can explicitly setup setters/getters for the controller with something like: reactive_attribute :name, but the _ saves quite more time than you might expect.

In some ways I see the _'s like scaffolding in rails. Popular at first, then slowly will go away. I think Volt's first niche will be building apps quickly, then evolve from there.

@bbatsov
Copy link
Contributor Author

bbatsov commented Feb 22, 2015

@bbatsov so on the swallowing of errors, in order to use a property before its been assigned, it has to return something when you ask for it, in our case we return nil. If it just threw an error or logged one, then you couldn't do bindings without initializing them first. That would add a lot more code.

I meant that we can do what we're doing right now and just add some extra tracking in method_missing - invocations to undefined methods for the first time and depending on reader/writer either return nil or initialize an attribute. Sure we'll swallow the errors in a way, but all the relevant info will still be in the log. Doesn't seem to me that this will add a lot of code, but I might be missing something you're aware of.

So if your just coming from rails, you might not fully understand why you can't just add fields explicitly all of the time. The main reason is that most fields get bound to properties on the controller. You can explicitly setup setters/getters for the controller with something like: reactive_attribute :name, but the _ saves quite more time than you might expect.

Perhaps it does. As I said my primary issues with the naming convention choice, not with the feature itself. I don't quite understand the Rails remark - after all there the attributes are usually generated directly from the schema. It's not like people are declaring attributes manually all the time.

In some ways I see the _'s like scaffolding in rails. Popular at first, then slowly will go away. I think Volt's first niche will be building apps quickly, then evolve from there.

Maybe.

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

It's anything but natural. Overloading existing semantics on a per-project basis is rarely a good idea. One can get used to this, but should they?

I meant in the context of attributes - it looks like a normal attribute

That's not a property of using _ per se, but of using some convention.

I agree, the 'var_' part is the indicator.

That's super subjective.

I'm not so sure, Ruby has a 'style' (which attracts many of us to it) and part of the decision process should be, does this feel like Ruby? Yes will probably = nice.

In my book explicit beats implicit most days of the week

&

In some ways I see the _'s like scaffolding in rails. Popular at first, then slowly will go away. I think Volt's first niche will be building apps quickly, then evolve from there.

I agree that we should probably shift focus away from the _'s in the docs and learning material, and only use when it's not feasible otherwise. It's probably the most confusing thing about Volt at the moment.

@ryanstout
Copy link
Member

@AstonJ So you think we should show using the field in the demos?

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

Yeah, definitely - it lowers the barrier significantly imo.

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

We could always add a note to say there's a shortcut for those who want to very, very quickly prototype something. This will help show Volt has neat tricks up its sleeve for hardcore Volt'ers :D

@ryanstout
Copy link
Member

@AstonJ I think when you build a few more apps you'll see a bit more why its so useful. There's a lot of times you want to bind, then use the value elsewhere. It's less useful on models directly (where I would probably define fields than on collections like page)

@ryanstout
Copy link
Member

@AstonJ I'm not really sold on var_, I think I would be more likely to do: model[:name] instead.

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

@ryanstout Sure, both (and v_something) are better than the plain underscore imo. They're just not as jarring or awkward looking.

I think when you build a few more apps you'll see a bit more why its so useful. There's a lot of times you want to bind, then use the value elsewhere. It's less useful on models directly (where I would probably define fields than on collections like page)

I can definitely see its uses, and hopefully switching to [] (or some other alternative) will resolve these issues.

@andrew-carroll
Copy link

I like #get and #set, personally. To keep the ease of refactoring afforded by _, I suggest calling get(:item) should call the get_item method if it exists, and behave as _item does now if it doesn't. So I can have get(:name), which will mimic _name normally, but if I want to change the implementation later I can do:

# index.html
<p>{{ get(:name) }}</p>

# main_controller.rb
...
private
def get_name
  get(:first_name) + ' ' + get(:last_name)
end

def set_name(value)
  first, last = value.split(' ')
  set :first_name, first
  set :last_name, last
end

@andrew-carroll
Copy link

Hrm, although that breaks computations. Oh well.

@AstonJ
Copy link

AstonJ commented Feb 22, 2015

Here are the other variations:

class Person < Volt::Model
  def name=(name)
    self._first_name, self._last_name = name.split(' ')
  end

  def name
    _first_name + ' ' + _last_name
  end
end
class Person < Volt::Model
  def name=(name)
    self.v_first_name, self.v_last_name = name.split(' ')
  end

  def name
    v_first_name + ' ' + v_last_name
  end
end

I'm not sure which I prefer now...

@lexun
Copy link
Contributor

lexun commented Feb 22, 2015

The underscores have grown on me personally, in spite of the community convention of using them to mark unused arguments. But if a change is in order, I think I'm with @andrew-carroll as far as #get and #set making the most sense at this point...

@ryanstout
Copy link
Member

@lexun Yea, the _'s grow on you :-) I'm not sure I could get behind get and set, it's just a lot of typing. That said, let me think if I can come up with a better solution. I think for me its between _ and []

@andrew-carroll
Copy link

Another alternative: #bind. Like so:

# index.html
<p>{{ bind :name }}</p>

# volt/lib/volt/models/model.rb
def bind(attribute, value=nil)
  if respond_to? :"bind_#{attribute}"
    send(:"bind_#{attribute}", value)
  else
    assign_attribute(attribute, value) if value
    read_attribute(attribute)
  end
end

# main_controller.rb
...
private
def bind_name(name=nil)
  first, last = name.split(' ') if name
  bind(:first_name, first) + ' ' + bind(:last_name, last)
end

@andrew-carroll
Copy link

I agree that it's a bit more typing, and it'd be nice to avoid that, but I don't think the behavior of prefixed underscores is obvious. Between _ and [ ], I am much fonder of [ ]; however, I think that still suffers from a "What does that do?" problem.

@AstonJ
Copy link

AstonJ commented Feb 23, 2015

I have to admit, looking at the alternatives, underscores are growing on me too.

Are suffixes worth looking at?

class Person < Volt::Model
  def name=(name)
    self.first_name_, self.last_name_ = name.split(' ')
  end

  def name
    first_name_ + ' ' + last_name_
  end
end

I think these are probably my favourite, followed by prefixes.

However to be perfectly honest there's not much in it, and I'm happy to support whatever Ryan deems best.

@bbatsov
Copy link
Contributor Author

bbatsov commented Feb 23, 2015

Btw, another alternative - we can do something like:

model.auto.attr_name

Might be a bit more clear than prefixing all the prototype attr method names.

@ryanstout
Copy link
Member

Some good ideas here guys. Let me think a bit (and keep the ideas coming). I agree I think there's probably a better way to do things, I just don't want to make things too verbose.

@AstonJ
Copy link

AstonJ commented Feb 23, 2015

model.auto.attr_name

Nice idea 👍

Another alternative: #bind

Growing to like this too.

I just don't want to make things too verbose

Agree with that as well.

@bbatsov
Copy link
Contributor Author

bbatsov commented Feb 25, 2015

One more argument in favour of []:

Models act like a hash that you can access with getters and setters that start with an underscore. If an attribute is accessed that hasn't yet been assigned, you will get back a "nil model". Prefixing with an underscore makes sure that we don't accidentally try to call a method that doesn't exist. Instead, we get back a nil model instead of raising an exception. Fields behave similarly to a hash, but with different access and assignment syntax.

Btw, another idea I recently had was using __ as a prefix, but it's not easy no discern the two _ in many fonts and I'd still prefer a convention that doesn't start with _ at all.

@dfl
Copy link

dfl commented Mar 12, 2015

@bbatsov +1 for hash syntax. Goes with the Ruby's Principle Of Least Surprise.
model.auto.attr_name is also a viable alternative, if the goal is for placeholding and easy removal later.

@gfowley
Copy link

gfowley commented Mar 25, 2015

Preventing name collisions for an object whose method names will be assigned at runtime is tricky. My approach has been to prioritize the user-developer experience (often my future self)...

It is possible to reserve most of the non-prefix method namespace for users' attributes by having the Model class inherit from BasicObject to minimize existing methods. Then prefix names for all non-attribute methods. For example with _:

model._attributes
model._save
# etc...

FWIW: my case was for a dynamic value-object so there were very few non-attribute methods.

During prototyping (no field specified in Model class) users could do:

model.name
model.name=

...implemented with method_missing.
Pros: no ceremony, looks like a normal attr_accessor .

After protyping (field is specified in Model class) users could do:

model.name
model.name=

...implemented with a defined method (class_eval, define_method, whatever...).
Pros: no change from prototype, defined method is performant, still looks like a normal attr_accessor.

And for convenience, also support:

model[:name]
model[:name]=
model["name"]
model["name"]=

... all very Ruby-ish. Easy wins could also include:

# Hash#fetch semantics
model.fetch(:name)
# getters/setters
model.get(:name)
model.set(:name)  

@mileszs
Copy link

mileszs commented May 14, 2015

I must agree that something other than the underscore would be great. Even without the baggage associated with underscore preceding a variable or the like, it is confusing, in my opinion. I tend to appreciate a bit more verbosity seeking clarity, so model.fetch or the like appeals to me.

We had a demo of Volt at the Indianapolis Ruby Brigade meeting today. I came across this discussion after a question was asked about the underscore idiom. There was much confusion.

@brandonmikeska
Copy link

Won't this feature only work with databases that are schemaless? I am not a huge fan of the underscore either but I am just curious when it comes to other data stores?

@rubydesign
Copy link

just another 2 pence: Don't use the underscore if you don't like it!!
The code has Model get / set so anyone can use that if they like to type, or feel it is more expressive.

That and the fact that you can always use fields makes this more a discussion about how other people should code, ie fruitless.

I think it it is a good and simple shortcut that ryan has created. I would much much rather see {{ _name }}, than {{ model.get(:name) }} but each to his own. Off course {{ name}} is better still, and i would use fields after prototyping.

So maybe the whole discussion boils down to a documentation issue. Maybe it is not the first feature one should advertise and rather start with the fields approach.

@vais
Copy link

vais commented Jul 13, 2015

I think @ryanstout's idea about using underscores in the prototyping phase, then removing them as the fields are explicitly added to models is nothing short of brilliant. This is known as syntactic vinegar - it will look ugly until you do the right thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests