github / twui

A framework for developing interfaces on the Mac.
Other
400 stars 79 forks source link

Refactoring and renaming. #54

Closed avaidyam closed 12 years ago

avaidyam commented 12 years ago

I suggest we add categories to NSColor and NSImage and the like to match the long-lost TUIImage and TUIColor, which had simpler method names and more. I also suggest we rename a few classes such as TUINSView and NSTUIView to something that makes more sense and keeps a TUI* prefix.

jwilling commented 12 years ago

I would argue that those categories should not be implemented into TwUI, mainly because you're trying to make the framework do too much. I think putting those in a separate library might be a nice addition, and even if others aren't using TwUI they can still take advantage of them.

avaidyam commented 12 years ago

Well, I meant, for instance, calling -[NSColor colorWithRed:Blue:Green:] instead of -[NSColor colorWithCalibratedRed:Blue:Green:] or the like. It's just ease-of-use. Sure, the special methods that do amazing things could be dropped, but only if they're longer than a few lines.

jspahrsummers commented 12 years ago

I don't think the convenience is worth the tradeoff of being incompatible with/different from all the AppKit code that is already out there. There's always the ability to add categories at the application level, or, like @jwilling proposed, in a third-party library, but I don't see the value in adding them to TwUI.

Also, there are no view classes without a TUI prefix. I'm not sure what you're proposing to rename.

avaidyam commented 12 years ago

@jspahrsummers We could just map one method to the other....? So your apps can use either. This helps especially if you're still on an older TwUI (not sure if it's called a version).

I was trying to say we need to rename the container classes which bridge between TUIView and NSView to make more sense.

jspahrsummers commented 12 years ago

Well, I think backwards compatibility kind of went out the window when I started introducing those changes. I don't want to make any steps in that direction, because it's pointless unless we're going to maintain full compatibility (which we aren't).

As for mapping one method name to the other, duplicate facilities are always a Bad Idea™. It just introduces confusion and cognitive load that no one needs. Again, I would recommend doing this at the application level if you're sure you want it.

What sort of renaming did you have in mind?

avaidyam commented 12 years ago

Well, alright then. I was going to use TUIContainerView and TUIAdapterView to start. We do need to clarify more in the names though, than what I've come up with.

jwilling commented 12 years ago

Although that looks nicer, the problem I have with getting rid of NS is that it's not immediately obvious what type of class it is. For example, TUIContainerView. That doesn't really make immediate sense from a quick glance. Is that a TUIView within a TUIView, is it a NSView within a TUIView, or is it even a TUIView within a NSView?

They're currently awkwardly named, but it's more descriptive that way.

jspahrsummers commented 12 years ago

I do think TUIViewNSViewContainer is horrible, but I don't know what to use instead. TUINSView seems relatively unambiguous — I don't think that one needs to change.

avaidyam commented 12 years ago

TUIAdapterNSView and TUIAdapterTUIView? We'd have to remodel the TUIAdapter* classes to really be transparent then. But it works, and it really does signify the purpose of the class.

jspahrsummers commented 12 years ago

Does TUIAdapterNSView adapt an NSView to TwUI or the other way around? Still not a net increase in clarity.

avaidyam commented 12 years ago

Alright, how about TUIHostNSView and TUIHostTUIView? The ending *View would be what class it hosts, so TUIHostNSView would be a TUIView hosting an NSView and likewise for the other.

jwilling commented 12 years ago

That seems backward to me, at least.

avaidyam commented 12 years ago

Alright, any other proposals? I'm on the same thought train with my names here.

jspahrsummers commented 12 years ago

Honestly, I think any parallel naming scheme is going to lead to confusion. Right now, TUINS* unambiguously means "something hosted by AppKit," and TUIViewNSViewContainer is so verbose that its purpose is unlikely to be forgotten, and it's not liable to be confused with TUINSView.

Also, I'm starting to think that TUIViewNSViewContainer being so verbose is a good thing if it discourages use. AppKit bridging should only really be used when necessary, because of how magic it is.

avaidyam commented 12 years ago

HAHA, Alright, if it's too magic, I guess a verbosely annoying name can dissuade its use. So, then naming will be kept as is? In regards to methods, i'd request just two methods to be added, it's only 5 lines more: + [NSColor colorWithWhite:alpha:] and + [NSColor colorWithRed:green:blue:alpha:]. In fact, they just map the method to an existing method. Just two methods. Nothing more.

jspahrsummers commented 12 years ago

Correct, the naming will remain the same unless I hear or think of something better than TUIViewNSViewContainer.

See my above comment regarding duplicate APIs.

In short, if both colorWithWhite:alpha: and colorWithCalibratedWhite:alpha: (and RGB methods) exist, people are going to wonder what the difference is. There's no need to introduce that kind of confusion, however minor, if we don't derive a great benefit from it. Again, this seems more appropriate for the application level if you find the tradeoffs worth it.

dannygreg commented 12 years ago

I don't understand the benefit of that addition at all to be honest. We have autocomplete… so it's not even going to make much difference when typing!

On 24 Aug 2012, at 04:26, Justin Spahr-Summers notifications@github.com wrote:

Correct, the naming will remain the same unless I hear or think of something better than TUIViewNSViewContainer.

See my above comment regarding duplicate APIs.

In short, if both colorWithWhite:alpha: and colorWithCalibratedWhite:alpha: (and RGB methods) exist, people are going to wonder what the difference is. There's no need to introduce that kind of confusion, however minor, if we don't derive a great benefit from it. Again, this seems more appropriate for the application level if you find the tradeoffs worth it.

— Reply to this email directly or view it on GitHub.

avaidyam commented 12 years ago

Guess it's just me being OCD again, then. :]