Skip to content

Instantly share code, notes, and snippets.

@phiggins
Created July 8, 2010 06:44
Show Gist options
  • Save phiggins/467714 to your computer and use it in GitHub Desktop.
Save phiggins/467714 to your computer and use it in GitHub Desktop.

Jul 07, 14:01:23  seacreature: I will be starting in 2 minutes, but please say hi now and mention your city :)

Jul 07, 14:01:24  locks: or we'd have the sessions up and running

Jul 07, 14:01:29  chrisdinn: JOINED

Jul 07, 14:01:56  locks: hello, I'm ricardo from Porto, Portugal

Jul 07, 14:02:03  chastell: hi, this is Piotr from Warsaw, Poland

Jul 07, 14:02:10  fwoeck: Hi! Frank here, from Frankfurt (really), Germany 23:00 CST

Jul 07, 14:02:19  stuarte: hello, I'm Stuart from Newtown, UK

Jul 07, 14:02:27  etrepat: Hi! I'm Estanislau from Lleida, Spain

Jul 07, 14:02:27  tjsingleton: TJ from Atlanta, GA, US

Jul 07, 14:02:31  mhfs: hey there, Marcelo from Campinas, BR

Jul 07, 14:02:35  chapambrose: Chap in Philly, US

Jul 07, 14:02:35  tundal45: this is Ashish from Wilton, CT, USA

Jul 07, 14:02:38  aguids: hi, Felipe from Salvador Brazil

Jul 07, 14:02:46  anteaya: Anita from Ontario, Canada

Jul 07, 14:02:47  sweetmango: JOINED

Jul 07, 14:02:50  andres-fc: howdy, Andres from Jalisco, Mexico

Jul 07, 14:03:00  volundr: Hi, Walton from Boise, ID (USA)

Jul 07, 14:03:08  sweetmango: hi, Jia from new haven, ct

Jul 07, 14:03:11  neXter:_ JOINED

Jul 07, 14:03:13  ddux: Dakota, Madison, WI

Jul 07, 14:03:13  afcapel: Hi, I'm Alberto from Madrid, Spain

Jul 07, 14:03:37  chrisdinn: IChris from Toronto, Canada

Jul 07, 14:03:54  chrisdinn: hey everyone!

Jul 07, 14:04:00  morgy: Hi, Morgan, Edinburgh

Jul 07, 14:04:06  carlosantonio: hi all

Jul 07, 14:04:12  lightalloy:_ Hello

Jul 07, 14:04:27  neXter:_ Hi, I'm Pieter from Berlin, Germany (just came back from public viewing the soccer game), hi to everybody!

Jul 07, 14:04:38  christiangeek: gabe from Grand Rapids MI

Jul 07, 14:04:40  carlosantonio: I'm from Rio do Sul, Brazil

Jul 07, 14:04:43  pleax: Hey! I'm Dmitry from Saint-Petersburg, Russia

Jul 07, 14:04:44  seacreature: (those who still are introducing keep going, but for everyone awake, this is where the code we'll be reviewing lives: http://github.com/rmu/rmu-entrance-exam-2010 )

Jul 07, 14:04:58  lightalloy:_ I'm Ann, from Vologda, Russia

Jul 07, 14:05:31  seacreature: andres-fc: are you here?

Jul 07, 14:05:37  mhfs: QUITED

Jul 07, 14:05:39  andres-fc: yessir

Jul 07, 14:05:47  seacreature: oh I see, got lost up there :)

Jul 07, 14:05:54  sweetmango: to neXter_ , sorry about the game

Jul 07, 14:06:23  seacreature: Folks, andres-fc has volunteered to do a bit of journalism for us, hopefully you saw the first RMU Insider

Jul 07, 14:06:38  anteaya: link?

Jul 07, 14:06:50  seacreature: getting it...

Jul 07, 14:06:55  seacreature: http://blog.majesticseacreature.com/20683384

Jul 07, 14:07:05  anteaya: ta

Jul 07, 14:07:24  seacreature: I'm pretty sure andres-fc watches the list, but if you have an announcement about RMU you want to get out, it's a good way to do it

Jul 07, 14:07:51  seacreature: just contact him and he can summarize periodically

Jul 07, 14:07:59  anteaya: thanks andres-fc, very kind of you

Jul 07, 14:08:03  andres-fc: Like seacreature said, just shoot me up an email and we'll talk about it

Jul 07, 14:08:22  mhfs: JOINED

Jul 07, 14:08:23  seacreature: also, though I won't be able to pick you all out by irc handle, if you've organized or participated in an RMU focus group, fantastic!

Jul 07, 14:08:46  semmons99: JOINED

Jul 07, 14:08:47  seacreature: I didn't anticipate a community growing before classes started, but seeing that happen has been deeply inspiring

Jul 07, 14:09:06  seacreature: so thank you for making this a great thing so far.

Jul 07, 14:09:21  tundal45: thank you for enabling it!

Jul 07, 14:09:47  anteaya: ^

Jul 07, 14:09:53  seacreature: If anyone wants to help take notes throughout this session, I've found etherpad to be great for that

Jul 07, 14:09:55  fwoeck: yes defntly - great idea and effort!

Jul 07, 14:10:05  semmons99: QUITED

Jul 07, 14:10:06  seacreature: http://ietherpad.com/uKTfbwBnMb

Jul 07, 14:10:54  tundal45: i'm at work so I won't be able to give 100% attention

Jul 07, 14:10:56  seacreature: We're about to get down to business, but before we do, I want to take a quick poll of whether folks would be okay with this transcript being open to the public for this event.

Jul 07, 14:11:00  seacreature: tundal45: no worries

Jul 07, 14:11:07  locks: http://piratepad.net/ arr maties

Jul 07, 14:11:16  chapambrose: no problem here

Jul 07, 14:11:20  pleax: it's ok

Jul 07, 14:11:21  sweetmango: fine with me

Jul 07, 14:11:23  morgy: no problem for me

Jul 07, 14:11:23  seacreature: If anyone has issues with that, please publicly or private message me

Jul 07, 14:11:25  carlosantonio: no problem at all

Jul 07, 14:11:30  tundal45: no problem for me either

Jul 07, 14:11:31  neXter:_ np for me

Jul 07, 14:11:37  seacreature: otherwise, we don't need to peer pressure everyone :)

Jul 07, 14:11:42  christiangeek: no problem

Jul 07, 14:11:43  stuarte: OK

Jul 07, 14:11:48  andres-fc: heheheh

Jul 07, 14:11:49  aguids: ok

Jul 07, 14:11:50  afcapel: no problem for me

Jul 07, 14:11:53  fwoeck: sure, no prob

Jul 07, 14:11:59  etrepat: ok for me too

Jul 07, 14:12:05  chrisdinn: no problem

Jul 07, 14:12:08  volundr: fine by me

Jul 07, 14:12:11  imsaar: JOINED

Jul 07, 14:12:16  seacreature: I'll give a minute for anyone to disagree. Otherwise, go change your nick to something creepy for anonymity

Jul 07, 14:12:45  locks: public for me

Jul 07, 14:13:16  seacreature: Okay. sounds like we're going public with this.

Jul 07, 14:13:32  seacreature: In general I want to keep our interactions private just because I like the smallness of the community

Jul 07, 14:13:39  seacreature: but I want to also validate the group to the public

Jul 07, 14:13:50  seacreature: so doing things like this with open transcript is nice

Jul 07, 14:14:03  seacreature: there will be a mixture of private and public content in regular sessions.

Jul 07, 14:14:18  seacreature: anyway, that's all for our meta-discussion. Not too bad for our first meeting I hope

Jul 07, 14:14:26  bradcantin: JOINED

Jul 07, 14:14:38  seacreature: does anyone have any immediate questions itching at them?

Jul 07, 14:14:49  seacreature: If not, I'll go through some solutions one by one and discuss them as a group

Jul 07, 14:14:50  locks: nope

Jul 07, 14:15:34  carlosantonio: ok

Jul 07, 14:15:42  seacreature: Okay, cool. We can do plenty of general Q&A in the end if people need it

Jul 07, 14:16:09  seacreature: Please take a minute or two to look at Tim Liner's solution now.

Jul 07, 14:16:11  seacreature: http://github.com/rmu/rmu-entrance-exam-2010/blob/master/lib/tim_liner.rb

Jul 07, 14:16:28  randuin: JOINED

Jul 07, 14:16:31  seacreature: Tim, are you here?

Jul 07, 14:17:16  seacreature: maybe not... but this is why the acceptance form asked for permission to display solutions :)

Jul 07, 14:17:39  seacreature: Anyway, folks who have already reviewed this solution, please discuss your thoughts on it

Jul 07, 14:17:45  seacreature: others who are reading now can catch up

Jul 07, 14:18:02  seacreature: I can certainly offer my thoughts as well, but I want to see where the group discussion leads first

Jul 07, 14:18:10  pleax: don't like make_change

Jul 07, 14:18:29  seacreature: pleax: elaborate?

Jul 07, 14:18:55  volundr: it

Jul 07, 14:19:00  carlosantonio: I guess make_change could be refactored, because it is responsible to both actions

Jul 07, 14:19:13  anteaya: for those of use just looking at the code for the first time, can you tell us what make_change does and what you don't like about it?

Jul 07, 14:19:15  carlosantonio: in this case, undo e redo

Jul 07, 14:19:25  seacreature: (btw, folks who haven't been following the list, lots of background here: http://groups.google.com/group/ruby-mendicant-university----general/browse_thread/thread/ca1a0740077d064 )

Jul 07, 14:19:26  carlosantonio: (or add and remove)

Jul 07, 14:19:31  volundr: it's quite verbose, there's a lot going on there and it's hard to seperate it out. It took me a couple reads to "get it"

Jul 07, 14:19:33  pleax: all logic in one place. better to split areas of responsibilities by action (add/remove)

Jul 07, 14:19:48  afcapel: yes, nested if, else, unless... smell like the method is doing to much

Jul 07, 14:19:49  anteaya: pleax, thanks

Jul 07, 14:20:08  stuarte: I'd be interested to see Tim's test

Jul 07, 14:20:14  aguids: make_change has to figure out what the command is to undo it

Jul 07, 14:20:15  seacreature: (short URL: http://is.gd/djaX3 )

Jul 07, 14:20:18  jcangas: JOINED

Jul 07, 14:20:50  fwoeck: just from staring at the code, i'd say it's procedural

Jul 07, 14:20:52  tundal45: i had a similar solution to Tim's & my version of make_change was even worse

Jul 07, 14:21:05  seacreature: oh, stuarte good point

Jul 07, 14:21:15  seacreature: ALL RESULTS OF BENCHMARKS AND TESTS: http://github.com/rmu/rmu-entrance-exam-2010/blob/master/results.txt

Jul 07, 14:21:28  aguids: mine was similar too

Jul 07, 14:21:35  anteaya: tundal45, why make one large method? did you have a reason?

Jul 07, 14:22:13  tjsingleton: seacreature: what vm?

Jul 07, 14:22:22  imsaar: for methods like make_change ad snapshot taking in a hash of arguments would make it more readable

Jul 07, 14:22:27  carlosantonio: there is also logic some duplication, by recreating the same actions in make_change

Jul 07, 14:22:28  seacreature: --- I think I'll let discussion go 5-10 minutes on each solution, then spend 2-3 minutes summarizing and sharing my thoughts, 2-3 minutes Q&A about my discussion, repeat ----

Jul 07, 14:22:33  tundal45: i think I was just not thinking to clean it up too much

Jul 07, 14:22:40  tundal45: i kept on seeing submissions pouring in

Jul 07, 14:22:45  tundal45: so once i got the tests to pass

Jul 07, 14:22:50  tundal45: i decided to submit mine

Jul 07, 14:23:02  seacreature: tjsingleton: likely ruby 1.8.7 on OS X on a MBP, cant remember because I installed rvm and I have .rvmrc's all over the place

Jul 07, 14:23:11  andres-fc: I personally feel that passing the snapshots array and modifying it inside the method is dangerous

Jul 07, 14:23:15  seacreature: but they're all on the same platform, same implementation, at least :)

Jul 07, 14:23:20  anteaya: tundal45, ah time pressure, okay

Jul 07, 14:23:22  locks: I get you tundal45, I submitted mine with artifacts and everything

Jul 07, 14:23:43  tundal45: that & I am a super n00b

Jul 07, 14:23:58  seacreature: --- another 2 minutes on this and then I'll summarize ---

Jul 07, 14:24:00  tundal45: so anything additional would have required a lot of thought & possibly some research

Jul 07, 14:24:56  andres-fc: tundal45: Don't worry about it, I think most of us felt the time pressure and understand you :)

Jul 07, 14:25:06  anteaya: so other than make_change is too big and has too much responsibility, is there any thing else to be said about this solution?

Jul 07, 14:25:21  pleax: it's minor, but: "length, position = change" :)

Jul 07, 14:25:33  pleax: unpack i mean

Jul 07, 14:25:46  anteaya: pleax, line number?

Jul 07, 14:25:54  chastell: pleax: that'd be "_, length, position = change"

Jul 07, 14:26:07  carlosantonio: also, snapshot is not being used without tainted option (false)

Jul 07, 14:26:08  pleax: chastell: oh, yes)

Jul 07, 14:26:20  seacreature: --- okay, please wrap up with your last thoughts on this now ---

Jul 07, 14:26:38  seacreature: Great.

Jul 07, 14:26:43  carlosantonio: instead of snapshot, make_change adds to the array

Jul 07, 14:26:45  bradcantin: while make_change is a little long, and can be refactored, there is nothing wrong with having one place for the responsiblity for undo / redo

Jul 07, 14:27:18  seacreature: So, I feel like I don't need to do anything. You folks can teach yourselves. Great conversation so far.

Jul 07, 14:27:28  seacreature: But maybe I can help a little bit afterall :)

Jul 07, 14:27:42  stuarte: It's OK, but would be dangerous if you kept developing

Jul 07, 14:27:45  tundal45: im not too sure about that </speaking for myself>

Jul 07, 14:27:49  pleax: it will be hard to maintain make_change with all these if-elsif when new commands being introduces

Jul 07, 14:27:57  stuarte: Two reponsibilities could become three

Jul 07, 14:28:01  stuarte: and so on

Jul 07, 14:28:03  seacreature: PLEASE LET ME TALK NOW :)

Jul 07, 14:28:07  locks: haha

Jul 07, 14:28:15  locks: voice the channel seacreature ;)

Jul 07, 14:28:18  locks: we won't mind

Jul 07, 14:28:29  seacreature: trying to organize without that.

Jul 07, 14:28:41  seacreature: But this is a learning process for all of us, no worries

Jul 07, 14:28:49  seacreature: Okay, so to summarize here.

Jul 07, 14:28:56  seacreature: I completely agree about most of what was said here

Jul 07, 14:29:05  seacreature: make_change is a monolithic function, or "god function"

Jul 07, 14:29:35  seacreature: When we have code like this, even though add_text and remove_text appear as different features from the API

Jul 07, 14:29:44  seacreature: we didn't actually abstract anything by separating them out

Jul 07, 14:29:52  seacreature: because make_change does not generalize.

Jul 07, 14:30:16  seacreature: It simply contains the logic for both features, and has two entry points that determine which cases get executed.

Jul 07, 14:30:41  seacreature: But I have to say, this is what maybe 30-40% of all submissions looked like.

Jul 07, 14:31:03  seacreature: And that's really okay. Often times, it actually does more harm than good to worry about single-responsibility up front

Jul 07, 14:31:09  seacreature: it's a lot easier when it smacks you in the face.

Jul 07, 14:31:22  seacreature: A good way to run into that head first is to attempt to test this code

Jul 07, 14:31:45  seacreature: make_change is sort of a private method, and it's where all the meat lies

Jul 07, 14:32:04  seacreature: if you test it directly, it's going to be very hard to write tests that seem to be grouped around a single concept

Jul 07, 14:32:31  seacreature: if you test it indirectly, you're keeping track of two things at once while you're testing, because you have to explicitly avoid the cases that are part of add_text and not remove_text, and vice versa

Jul 07, 14:32:52  seacreature: and as some of you pointed out, the real danger is when we go from 2 behaviors to n behaviors

Jul 07, 14:33:08  seacreature: Once you head down that road, these sorts of methods become very hard to maintain

Jul 07, 14:33:38  seacreature: All this having been said... if you wrote like this because you heeded my advice to stick within your area of comfort and only spend an hour or two

Jul 07, 14:33:41  seacreature: totally fine.

Jul 07, 14:34:10  seacreature: I still occassionally run into the same issue when I'm spiking. Not super important to immediately get this right, important to notice and refactor before it gets bigger.

Jul 07, 14:34:21  seacreature: -- OK, my little rant is done. Questions? ---

Jul 07, 14:34:44  aguids: shouldn't we only test the interface?

Jul 07, 14:34:54  aguids: you mentioned testing make_change directly

Jul 07, 14:34:54  chastell: I'm not sure what you meant by 'if you test it indirectly, you're keeping track of two things at once'

Jul 07, 14:35:01  tundal45: so separation of concerns is the end goal but it may not necessarily be the main focus early on?

Jul 07, 14:35:16  seacreature: Okay, let's go with chastell and aguids here

Jul 07, 14:35:18  chastell: i.e., if you make make_change private, I don't see how the testing differs from any other solution

Jul 07, 14:35:36  seacreature: This is because I wrote the tests already, and had an existing implementation in place

Jul 07, 14:35:55  seacreature: if you wrote the tests independently, it's unlikely that you would ever form a make_change method that does both things at once

Jul 07, 14:36:00  chastell: ah, I get it now; I'm too used to BDD to think about testing independently

Jul 07, 14:36:31  chastell: (sorry, I meant testing existing code)

Jul 07, 14:36:34  chastell: get it now

Jul 07, 14:36:58  seacreature: anteaya: yes?

Jul 07, 14:37:23  anteaya: just for a housekeeping thing, can we add the line number of the code when we comment about it

Jul 07, 14:37:38  imsaar: good ide anteaya

Jul 07, 14:37:39  seacreature: anteaya: not a bad idea

Jul 07, 14:37:40  imsaar: idea

Jul 07, 14:37:44  anteaya: thanks

Jul 07, 14:37:58  seacreature: we're talking about this http://github.com/rmu/rmu-entrance-exam-2010/blob/master/lib/tim_liner.rb#L39

Jul 07, 14:38:12  tundal45: github has awesome functionality of linking to the exact line

Jul 07, 14:38:14  tundal45: oops

Jul 07, 14:38:19  tundal45: beat me to it!

Jul 07, 14:38:21  seacreature: aguids: Does this answer your question?

Jul 07, 14:38:30  aguids: yep

Jul 07, 14:38:49  anteaya: lovely, thanks seacreature

Jul 07, 14:38:52  aguids: I missed the method wasn't private...

Jul 07, 14:38:59  seacreature: It sounds like we have some BDD diehards in here, and I hope to soften your zealotry by the end of your first RMU session :)

Jul 07, 14:39:26  chastell: :) sorry if I sounded like a zealot

Jul 07, 14:39:31  seacreature: "Testing the interface" is definitely what we want to do, but that does not mean that it doesn't matter what lies beneath that interface

Jul 07, 14:39:40  seacreature: you're supposed to be thinking about your tests, too.

Jul 07, 14:40:05  seacreature: If testing your interface becomes complicated, it means that you have an interface that requires rethinking

Jul 07, 14:40:12  seacreature: Let's form that into something a bit more solid

Jul 07, 14:40:15  seacreature: say you have this:

Jul 07, 14:40:27  seacreature: a function a(), and a function b()

Jul 07, 14:40:33  seacreature: both depend on c()

Jul 07, 14:40:54  seacreature: c() has a conditional statement flipped by a() and b(), each reaching a different external resource

Jul 07, 14:41:13  seacreature: you can test the interface of a(), but you need to mock considering b()'s needs

Jul 07, 14:41:16  seacreature: and vice versa

Jul 07, 14:41:18  seacreature: bad, bad news

Jul 07, 14:41:42  seacreature: that's why whenever you have joint dependencies on a function, it's very important to generalize the dependency

Jul 07, 14:42:20  seacreature: BTW, just teasing about the BDD thing, it's actually where a lot of the best ideas come from, but they require a bit more depth than what the words themselves imply

Jul 07, 14:42:44  seacreature: does anyone want to tackle the point I just made? Or are we ready for another solution?

Jul 07, 14:42:52  tundal45: in this case when update depends on add or remove to figure out how to update, how do we go about generalizing?

Jul 07, 14:43:06  seacreature: tundal45: we'll visit some solutions that do this nicely later

Jul 07, 14:43:07  tundal45: should we have separate updates for add & remove?

Jul 07, 14:43:17  chastell: I'm perfectly ok with teasing, the more the better ;) and thanks for the a()/b()/c() explanation, I envisioned quite a couple of (bad) cases now

Jul 07, 14:43:59  seacreature: Okay, wrapping up this point now. Though some people love zed and some hate them

Jul 07, 14:44:18  seacreature: Required reading on this topic: http://www.zedshaw.com/essays/indirection_is_not_abstraction.html

Jul 07, 14:45:04  seacreature: Okay, so I picked Tim's solution first because it's the most common one I saw.

Jul 07, 14:45:24  anteaya: I don't always agree with Zed's word choice but he has never steered me wrong about code

Jul 07, 14:45:37  seacreature: Next most common: http://github.com/rmu/rmu-entrance-exam-2010/blob/master/lib/alberto_fernandez_capel.rb

Jul 07, 14:45:46  locks: quick sidebar: check mongrel2, great and clean code

Jul 07, 14:45:51  seacreature: Please take a few minutes to read, and then a few more to discuss.

Jul 07, 14:45:58  imsaar: how do you do actions in IRC?

Jul 07, 14:46:04  locks: /me

Jul 07, 14:46:18  seacreature: I'll share my thoughts in about 10 minutes

Jul 07, 14:46:22  locks: I don't think they are logged imsaar

Jul 07, 14:46:56  tjsingleton: QUITED

Jul 07, 14:46:59  pleax: like this solution, used similar approach

Jul 07, 14:47:03  chrisdinn: This looks a lot like mine, which I cribbed closely from Design Patterns.

Jul 07, 14:47:18  locks: like seacreature said previously, it's textbook design pattern

Jul 07, 14:47:41  pleax: not quite shure if it's ok to store state in action object (30th line)

Jul 07, 14:47:42  chapambrose: How did you guys decide to break out the Add/Remove into seperate classes? I never seem to know

Jul 07, 14:47:47  etrepat: code is clear and easy to follow

Jul 07, 14:47:48  chrisdinn: Usually, I'll spend more time refactoring the "Design Pattern" approach into something a little less verbose

Jul 07, 14:48:08  locks: it makes up for quite more code but it's very clear

Jul 07, 14:48:16  aguids: pleax that's the text removed

Jul 07, 14:48:20  christiangeek: QUITED

Jul 07, 14:48:29  aguids: you need the info on future undos

Jul 07, 14:48:57  pleax: aguids: i understand what it is, but holding state in action object may be considered bad practice

Jul 07, 14:48:58  tjsingleton: JOINED

Jul 07, 14:49:35  tjsingleton: JOINED

Jul 07, 14:49:43  seacreature: pleax: please elaborate

Jul 07, 14:49:52  tjsingleton: QUITED

Jul 07, 14:50:06  seacreature: Also, Alberto, are you here?

Jul 07, 14:50:17  afcapel: yeah, i'm here

Jul 07, 14:50:19  WAA4YT9: PARTED

Jul 07, 14:50:28  afcapel: i concede it is verbose

Jul 07, 14:50:29  seacreature: afcapel: brave :)

Jul 07, 14:50:42  afcapel: but i think it is easly extendeable

Jul 07, 14:50:46  carlosantonio: would it be a good way to refactor the code, so undoing Add will actually execute Remove and vice-versa?

Jul 07, 14:50:48  afcapel: thnxs :)

Jul 07, 14:50:52  tjsingleton: JOINED

Jul 07, 14:51:13  afcapel: i mean, if you need another action, the requirements are clear

Jul 07, 14:51:16  stuarte: If anybody isn't up on Design Patterns, I think this is the "Command" pattern

Jul 07, 14:51:35  afcapel: you need an execute and an undo methods

Jul 07, 14:51:36  jcangas: carlosantonio, just tjis is my way :)

Jul 07, 14:51:39  chrisdinn: More on the pattern: http://en.wikipedia.org/wiki/Command_pattern

Jul 07, 14:51:50  carlosantonio: my initial implementation was pretty similar

Jul 07, 14:52:50  carlosantonio: afcapel: yeah, it has a well defined interface

Jul 07, 14:53:34  jcangas: @afcapel, yes but in this case the undo just match another command of the application

Jul 07, 14:54:22  afcapel: yes, but not necessarily if we have many commands

Jul 07, 14:54:28  neXter:_ i could image, that this solution is maybe a bit more memory consuming than the last one?

Jul 07, 14:55:01  stuarte: I was wondering that

Jul 07, 14:55:09  anteaya: jcangas, are you saying that you feel the method defined on line 33 is a repeat of a different method?

Jul 07, 14:55:17  volundr: 16812 vs 11672

Jul 07, 14:55:48  jcangas: I mean: we need the undo method, here can be implemented "delegating" in other existing command,

Jul 07, 14:56:14  carlosantonio: A better way to have a bit less memory consumption is to use clear instead of creating another Array, so @reverted = [] would be @reverted.clear

Jul 07, 14:56:15  jcangas: In general this is not the case, of course

Jul 07, 14:56:18  imsaar: afcapel: very nice, clean and readable code

Jul 07, 14:56:21  seacreature: --- Please finish any last thoughts now, I'll start summarizing in a moment ---

Jul 07, 14:56:22  anteaya: jcangas, in which other existing command?

Jul 07, 14:56:47  volundr: so 44% memory than the last solution

Jul 07, 14:57:00  carlosantonio: anteaya: I guess Remove#undo is quite the same as Add#execute

Jul 07, 14:57:07  jcangas: Remove == Add#undo, isnt

Jul 07, 14:57:11  seacreature: OKAYIMMANGONATALKNOW!!!!

Jul 07, 14:57:30  anteaya: carlosantonio, yes, very similar I agree, but are they replaceable?

Jul 07, 14:57:42  seacreature: Several of you pointed out that this is textbook design pattern, and it is

Jul 07, 14:57:45  seacreature: It's the command pattern

Jul 07, 14:57:46  tjsingleton: it was my favorite. I thought it was clear, and overall performant. (especially depending on the vm). It was my initial approach. I liked passing the buffer through the functions

Jul 07, 14:57:59  seacreature: but here's a trick about design patterns.

Jul 07, 14:58:14  seacreature: First, let me take a poll: how many of you have done a non-trivial amount of C++/Java/C#?

Jul 07, 14:58:34  locks: I think I fall in the trivial side

Jul 07, 14:58:35  jcangas: +1

Jul 07, 14:58:38  locks: just college experience

Jul 07, 14:58:38  fwoeck: bash anyone ? ;)

Jul 07, 14:58:41  pleax: +1

Jul 07, 14:58:41  afcapel: me too

Jul 07, 14:58:42  andres-fc: +1

Jul 07, 14:58:44  tundal45: just college for me

Jul 07, 14:58:46  chrisdinn: me

Jul 07, 14:58:48  aguids: +1

Jul 07, 14:58:53  neXter:_ if 7 years ago counts, than +1

Jul 07, 14:59:01  seacreature: Okay, many of you have.

Jul 07, 14:59:10  stuarte: +1

Jul 07, 14:59:12  seacreature: Here's a core confusion about design patterns that I even fall into.

Jul 07, 14:59:22  seacreature: The implementation of patterns do NOT encapsulate the idea

Jul 07, 14:59:34  seacreature: It's a bit like poetry or music in translation

Jul 07, 14:59:46  seacreature: doing a direct translation does not necessarily preserve the beauty

Jul 07, 15:00:03  seacreature: This is a case in which the beauty is clearly lost.

Jul 07, 15:00:13  seacreature: However, it's also the way that I did it, out of muscle reflex

Jul 07, 15:00:27  seacreature: This problem was originally slated for rubyproblems.com

Jul 07, 15:00:42  seacreature: and my solution looked nearly the same as afcapel's when I wrote it up

Jul 07, 15:00:58  seacreature: the only exception was that rather than explicitly repeat undo in each object

Jul 07, 15:01:03  seacreature: I did something like

Jul 07, 15:01:12  seacreature: RemoveTextCommand.new(...).execute

Jul 07, 15:01:23  seacreature: in my add_text's undo

Jul 07, 15:01:34  seacreature: Just so that I wasn't duplicating my logic

Jul 07, 15:01:57  seacreature: The downside is that this approach is still very high ceremony

Jul 07, 15:02:02  seacreature: seems like a lot of code to do a simple thing

Jul 07, 15:02:17  seacreature: the upside is that as afcapel pointed out, it provides a brilliant framework for extensibility

Jul 07, 15:02:35  seacreature: Very clear rules for arbitrary commands to be hooked in

Jul 07, 15:02:53  seacreature: Also, while verbose, easy to understand

Jul 07, 15:03:13  seacreature: The only trouble is like many direct design patterns, this is probably way too much power for the job

Jul 07, 15:03:34  seacreature: that said, this approach, exactly as written, can be viable in very complex applications, so don't forget about it

Jul 07, 15:03:52  seacreature: I have to call out pleax and directly disagree about command objects holding state being a bad thing

Jul 07, 15:03:59  seacreature: It's a feature, and a big one at that

Jul 07, 15:04:16  seacreature: commands can capture what they need to run, and that can be preprocessed before the commands are executed

Jul 07, 15:04:32  seacreature: Huge win. I have examples of this idea from what we're doing on Prawn's edge, that we may get to discuss later

Jul 07, 15:05:00  seacreature: again, not something you can see the value of in this simple example, but later on, the ability to pre-process your arguments before the command is executed is part of the brilliance of this.

Jul 07, 15:05:26  seacreature: The last point I have here is that I'm surprised no one pointed out Rails migrations

Jul 07, 15:05:30  seacreature: seem familiar to that?

Jul 07, 15:06:03  locks: I don't use rails, maybe that's why

Jul 07, 15:06:10  seacreature: --- Okay /monologue : Please respond with Q&A

Jul 07, 15:06:39  stuarte: Interesting to hear you say state in objects is good

Jul 07, 15:06:40  seacreature: locks: awesome, but I'm sure plenty folks here do :)

Jul 07, 15:06:47  chapambrose: if you have time, go further in detail about the command objects holding state, over my head

Jul 07, 15:07:01  stuarte: The fashion seems to be the other way - functional

Jul 07, 15:07:01  seacreature: Okay, sounds like both of you have a similar question

Jul 07, 15:07:07  seacreature: I didn't say state was good

Jul 07, 15:07:14  seacreature: I said encapsulating it in a command object is good

Jul 07, 15:07:31  seacreature: In general you want to minimize and manage state

Jul 07, 15:07:54  seacreature: But imagine instead, a situation in which you're encapsulating a bit of code that renders text to a page

Jul 07, 15:07:55  afcapel: well, i think i �agree 100%

Jul 07, 15:08:14  seacreature: initially, you just have raw text objects, with maybe a tag so that those can be identified

Jul 07, 15:08:23  seacreature: if you just execute all those commands, you get default styling

Jul 07, 15:08:38  seacreature: if instead, you pull all the text insert commands with a given tag, and change their font size

Jul 07, 15:08:43  seacreature: and then run them

Jul 07, 15:08:43  etrepat: QUITED

Jul 07, 15:08:44  aguids:_ JOINED

Jul 07, 15:08:59  seacreature: You've taken advantage of the fact that each command encapsulates the arguments used to run it

Jul 07, 15:09:00  pleax: seacreature: could find this fowler's article now and maybe I'm wrong, but he said that commands should hold immediate system state and should be immutable all their live cycle after it

Jul 07, 15:09:14  seacreature: pleax: Fuck Fowler then :)

Jul 07, 15:09:19  locks: haha

Jul 07, 15:09:19  pleax: after initializing i mean

Jul 07, 15:09:21  etrepat: JOINED

Jul 07, 15:09:21  pleax: =))

Jul 07, 15:09:31  seacreature: I still don't think that's true

Jul 07, 15:09:39  jcangas: I agree with your comments on Patterns and 'great cermony for little task' for this problem: so what is your alternative, I don't see ...

Jul 07, 15:09:52  aguids: QUITED

Jul 07, 15:09:54  seacreature: jcangas: we'll look at some other approaches

Jul 07, 15:10:06  pleax: you can "capture" needed information in beginning of life cycle and then just use it

Jul 07, 15:10:19  pleax: but anyway, no strong rules there

Jul 07, 15:10:22  seacreature: I think that if your commands are something that can benefit from post processing after the beginning of the cycle, the state is good

Jul 07, 15:10:50  seacreature: Go back to my example of taking a pre-rendered stock set of text and applying formatting before it is rendered

Jul 07, 15:11:01  seacreature: command pattern provides a nice mechanism for that

Jul 07, 15:11:09  chapambrose: so to bring it back to this example, the line in the execute command is affecting the state? http://github.com/rmu/rmu-entrance-exam-2010/blob/master/lib/alberto_fernandez_capel.rb#L60

Jul 07, 15:11:19  chapambrose: adding to the commands array?

Jul 07, 15:11:58  afcapel: but this line is not in the command's code

Jul 07, 15:12:00  seacreature: Oh wait... while I've been pontificating, I think I see the source of that comment

Jul 07, 15:12:09  seacreature: http://github.com/rmu/rmu-entrance-exam-2010/blob/master/lib/alberto_fernandez_capel.rb#L34

Jul 07, 15:12:34  seacreature: Now I don't like the side effects here :-/

Jul 07, 15:13:21  seacreature: But I feel like that discussion is a bit deeper than where I wanted to go with this.

Jul 07, 15:13:25  afcapel: why?

Jul 07, 15:13:26  volundr: side effects?

Jul 07, 15:14:03  seacreature: afcapel: Because I'd rather see the commands operate in such a way that rather than acting on a contents object

Jul 07, 15:14:14  seacreature: they do a transform and produce a fresh one

Jul 07, 15:14:35  seacreature: though because this is Ruby, that's a performance concern due to our horrible GC

Jul 07, 15:14:48  seacreature: So I don't really want to condemn this approach

Jul 07, 15:14:51  jcangas: seacreatue: Alreeady don, but imho, the command aproach seems more clean & elegant ...

Jul 07, 15:15:04  stuarte: I'd like to ask about the GC issue

Jul 07, 15:15:12  seacreature: jcangas: Again, I need to go back to the core problem

Jul 07, 15:15:18  seacreature: It isn't that command is wrong here

Jul 07, 15:15:23  seacreature: it's that a direct translation doesn't work

Jul 07, 15:15:35  seacreature: We'll look at an alternative in a moment

Jul 07, 15:15:39  afcapel: ok, this is something like functional programming, i mean inmutable objects.... isn't it?

Jul 07, 15:15:50  seacreature: stuarte: let's hit those at the end

Jul 07, 15:15:58  stuarte: Ok, thanks

Jul 07, 15:15:59  tjsingleton: I don't follow what you mean by "direct translation"

Jul 07, 15:16:16  etrepat: yeah I just got lost

Jul 07, 15:16:27  seacreature: Okay, so let's segway into ali rizvi's solution

Jul 07, 15:16:31  andres-fc: carbon copy without accounting for context, me thinks

Jul 07, 15:16:33  tundal45: i took it as ! vs non !

Jul 07, 15:16:44  seacreature: in interest of time, let's go with my fixed version

Jul 07, 15:16:46  seacreature: http://github.com/rmu/rmu-entrance-exam-2010/blob/master/lib/ali_rizvi_fixed.rb

Jul 07, 15:17:15  pleax: already said everithing in mailing list

Jul 07, 15:17:17  seacreature: tundal45: ooh, another issue we need to discuss. ! vs. non ! is not what you think :)

Jul 07, 15:17:31  seacreature: andres-fc: that is what I meant, yes

Jul 07, 15:17:47  seacreature: pleax: please take into account other participants

Jul 07, 15:18:03  seacreature: not everyone is reading the list actively, and not everyone has 'said everything'

Jul 07, 15:18:11  seacreature: we need to account for different opinions and skill levels here

Jul 07, 15:18:18  locks: my submission is pretty much == to ali's

Jul 07, 15:18:25  locks: especially after I cleanup mine today

Jul 07, 15:18:26  volundr: so one comment on Ali's solution was the expense of reading the contents

Jul 07, 15:18:35  locks: but he uses lambdas

Jul 07, 15:18:38  tundal45: i like the solution mostly because it stores actions and only changes contents when needed

Jul 07, 15:18:39  pleax: the problem i see here is every call of content methond leds to applying all stack of simple actions

Jul 07, 15:18:50  rgoytacaz: JOINED

Jul 07, 15:19:04  pleax: but solution looks very neat

Jul 07, 15:19:11  seacreature: --- I will jump in with comments in about 5 minutes ---

Jul 07, 15:19:12  andres-fc: right, which will only get worse as the document grows

Jul 07, 15:19:27  andres-fc: as pleax mentioned in the list, it has O(n) access time

Jul 07, 15:19:33  chapambrose: it sure is clean though, almost looks like it wouldn't work

Jul 07, 15:19:34  locks: I think it might have some scaling problems

Jul 07, 15:19:44  afcapel: i think reading is not an issue if you use caching

Jul 07, 15:19:52  volundr: http://github.com/Volundr/rmu-entrance-exam-2010/blob/master/lib/ali_rizvi_fixed_inc.rb addresses some of those problems by implementing a cache

Jul 07, 15:20:08  pleax: afcapel: it is if you read a lot

Jul 07, 15:20:14  volundr: and applying updates to that cache so that a simple edit doesn't invalidate the cache

Jul 07, 15:20:22  imsaar: yes as seacreatures said it depends on the use case

Jul 07, 15:20:24  pleax: afcapel: caching should be invalidated on every change

Jul 07, 15:20:32  morgy: What is happening on line 11: command.call ?

Jul 07, 15:20:33  andres-fc: au contraire, if you use a cache and read a lot, it's actually better

Jul 07, 15:20:48  imsaar: and I did heeded seacreatures advice and only spent 2 hours or less

Jul 07, 15:20:49  volundr: cache doesn't need to be invalidates for every change: see above link

Jul 07, 15:20:52  afcapel: ok, it could be an issue only with multiple read-write cycles

Jul 07, 15:21:05  etrepat: morgy: the stored block of code is executed, I thin

Jul 07, 15:21:19  locks: anyway, I'd be interested to know just how slow that contents method can be

Jul 07, 15:21:39  andres-fc: morgy: All the actions that need to be applied to the text are executed on that line

Jul 07, 15:21:50  andres-fc: morgy: then the actual contents are returned

Jul 07, 15:22:00  sweetmango: how does the code take care of the situation, that in undo you don't have to store the command in the list/

Jul 07, 15:22:01  chastell: I really like the approach with storing callable blocks (I wanted to submit such solution, but was late to the exam and didn't want to stall with my submission anymore), and if replays bite you performance-wise you can either cache or store undo/redo callable blocks instead

Jul 07, 15:22:02  jcangas: @seacreature: I get the message "adapt the pattern, using the features (idioms) of the target language" ( a block can already act as a Command). I'm ok?

Jul 07, 15:22:04  afcapel: i try no to think a lot in efficiency until i hit a real perfomance bottleneck

Jul 07, 15:22:15  chapambrose: why does execute clean the @reverted array to empty on every call (it's doing that right?) http://github.com/rmu/rmu-entrance-exam-2010/blob/master/lib/ali_rizvi_fixed.rb#L25

Jul 07, 15:22:20  aguids:_ QUITED

Jul 07, 15:22:23  seacreature: jcangas: yes, that's the core idea

Jul 07, 15:22:27  afcapel: (premature optimization is root of all evil)

Jul 07, 15:22:56  locks: chapambrose: because when you edit the text the undo stack is invalidated

Jul 07, 15:23:17  morgy: etrepat: & andres-fc Thanks. was expecting some thing like eval instead, never seen .call

Jul 07, 15:23:21  chapambrose: locks: thanks

Jul 07, 15:23:56  seacreature: --- Please finish your last thoughts ---

Jul 07, 15:24:11  jcangas: @seacreature: ok. Already know this and using in my ohter languages :), but nos so fluid in ruby

Jul 07, 15:24:37  seacreature: Okay, so, if you want to get some background on the particular problem about cache invalidation, definitely read the mailing list thread

Jul 07, 15:24:47  andres-fc: morgy: .call is what we have since directly using () is not supported

Jul 07, 15:24:52  seacreature: pleax pointed out a lot of things about efficiency for text editing

Jul 07, 15:25:10  seacreature: and context is quite important, because it helps determine what the right approach is

Jul 07, 15:25:19  seacreature: But consider by contrast the situation in Prawn.

Jul 07, 15:25:40  seacreature: We are adopting a delayed rendering system, so that we can walk Ruby objects and change anything about the document, only rendering finally on demand

Jul 07, 15:25:48  seacreature: in that case, the contents are only generated once

Jul 07, 15:25:56  seacreature: and because a lot of the raw PDF chunks can be cached

Jul 07, 15:26:07  seacreature: it is exactly the right approach to wait until the end to compose the commands

Jul 07, 15:26:15  imsaar: so was my implementation less literal implementation of the command pattern? as @seacreatures was pointing to?

Jul 07, 15:26:25  lightalloy: QUITED

Jul 07, 15:26:32  locks: imsaar: let's wait for the Q&A

Jul 07, 15:26:36  seacreature: imsaar: your implementation is totally command pattern, but could use some work

Jul 07, 15:26:43  imsaar: oops, sorry locks

Jul 07, 15:26:53  seacreature: for example, we can solve pleax's problem by added differential undos

Jul 07, 15:27:02  seacreature: but we could store those as procs as well.

Jul 07, 15:27:03  imsaar: @seacreature please elaborate

Jul 07, 15:27:10  seacreature: undo { remove_text(args) }

Jul 07, 15:27:32  imsaar: @seacreature: I did not quite understad what you mean by differential undos, please explain

Jul 07, 15:27:33  seacreature: that, combined with the idea of caching differentially, only when the content is rendered

Jul 07, 15:27:42  andres-fc: imsaar: Hang on :)

Jul 07, 15:28:00  locks: imsaar: a different undo for add and remove

Jul 07, 15:28:14  seacreature: so we can store what was changed.

Jul 07, 15:28:33  seacreature: now as to whether we lazily build up the content

Jul 07, 15:28:33  rgoytacaz: thats what i did, in mine.

Jul 07, 15:28:42  seacreature: or call the blocks immediately

Jul 07, 15:28:46  seacreature: that's a separate question

Jul 07, 15:29:11  seacreature: But you could use this general approach to implement command pattern without the need for classes

Jul 07, 15:29:29  seacreature: classes only arise when you start needing to mess with the state you built up those blocks with

Jul 07, 15:29:46  rgoytacaz: Hm... i see/

Jul 07, 15:29:48  seacreature: As for everyone who thinks that cache would solve this problem, please read the discussion on the list

Jul 07, 15:30:06  seacreature: pleax makes some good points, and you'd need a buffered input source to make caching feasible

Jul 07, 15:30:13  seacreature: just because think about it.

Jul 07, 15:30:28  seacreature: i just invalidated the cache like, what, 40 times in the sentence?

Jul 07, 15:30:34  seacreature: :)

Jul 07, 15:31:04  tjsingleton: ( according to my benchmarks the cached version performs worse)

Jul 07, 15:31:06  seacreature: so if you're calling .contents in a tight loop to update a display, that'd be an O(N) situation

Jul 07, 15:31:06  chastell: seacreature: so what are your thoughts on undo/redo callable blocks and current @contents?

Jul 07, 15:31:30  seacreature: chastell: one sec...

Jul 07, 15:31:52  seacreature: Anyway, I like to end things on time officially. So thank you all for coming, and if you have questions you can't get to now, follow up with us on the list

Jul 07, 15:31:58  seacreature: I am hoping for a hearty post-discussion

Jul 07, 15:32:09  seacreature: unofficially, I can be around for another 20 minutes for general discussion here

Jul 07, 15:32:22  seacreature: but no one should feel obligated to stay, we have transcripts

Jul 07, 15:32:34  tundal45: i need to bounce to get dinner & get out of the office before someone hands me more work

Jul 07, 15:32:42  seacreature: thanks for coming tundal45

Jul 07, 15:32:43  tundal45: but this was great

Jul 07, 15:32:46  locks: so you'll do the other submissions another day, seacreature ?

Jul 07, 15:32:51  tundal45: a lot of the stuff is still over my head

Jul 07, 15:32:57  tundal45: and i don't grasp fully

Jul 07, 15:33:04  tundal45: but im hoping to improve on taht

Jul 07, 15:33:18  seacreature: tundal45: no worries, this is not meant to be an instructional session as much as a way for us to get to know each other

Jul 07, 15:33:29  seacreature: and for me to evaluate the needs / interests of the group

Jul 07, 15:33:42  seacreature: I will send out an anonymous survey evaluating what people thought

Jul 07, 15:33:45  rgoytacaz: seacreature: One thing that I realized looking at most of the code, is that ppl didn't care about private some 'internal only` methods, isn't that a issue? exposing what isnt to be?

Jul 07, 15:33:59  tundal45: QUITED

Jul 07, 15:34:23  seacreature: before we continue with technical discussion, if anyone wants to comment publicly on this format of discussion and whether it's working for them, please do so now

Jul 07, 15:34:40  locks: I think it's mostly working

Jul 07, 15:34:46  locks: we are civilized enough :)

Jul 07, 15:35:20  locks: we could talk about some possible bot functions tho, seacreature, like silent voting or linkage, wtvr

Jul 07, 15:35:25  carlosantonio: I agree with locks

Jul 07, 15:35:26  chapambrose: i'm loving it, i don't know a lot of the standard CS or programming vocab, so I'm constantly googling, but this is good

Jul 07, 15:36:10  chastell: seems to be working quite well, maybe voicing will help in the future when seacreature wants to elaborate on something (but wasn't needed this time)

Jul 07, 15:36:44  anteaya: having a general lecture like this once a week would be great

Jul 07, 15:36:47  seacreature: great, everyone else, feel free to discuss publicly on list or via private email or message on irc if you're more comfortable that way

Jul 07, 15:36:49  anteaya: format works for me

Jul 07, 15:36:54  bradcantin: aye

Jul 07, 15:37:05  fwoeck: fine with me!

Jul 07, 15:37:05  seacreature: anteaya: the goal I have is to give problems for folks to work on and then discuss them in a format similar to this

Jul 07, 15:37:23  anteaya: seacreature, then I'm pleased, thank you

Jul 07, 15:37:39  afcapel: i think it is great so far. i just want more practical assignments...

Jul 07, 15:37:39  seacreature: Okay, great. now back to tech discussions

Jul 07, 15:37:48  morgy: seacreature: all these solutions look quite similar to me, storing diffs in memory etc my solution was to use the filesystem and have a persistant undo. Other than running really slow over the io did you have any comments on this type of approach?

Jul 07, 15:37:49  jcangas: Sorry I can read english & wirte it more or less, but Im not goog listen english & sure you don't want listen my spoken english :)

Jul 07, 15:38:11  etrepat: I agree with @afcapel, more practice would mean more fun ;)

Jul 07, 15:38:27  seacreature: btw, for those who are struggling with concepts, that's why we'll have a whole day of "Office Hours". So long as I can offset my costs of taking time away from work to do this, I want to be available for individual mentoring as much as possible

Jul 07, 15:38:44  volundr: @seacreature: I would be interested in some feedback on my edit to Ali's solution. It implements a cache that doesn't get invalidated on every edit.

Jul 07, 15:38:53  seacreature: the program hasn't started yet, so I can't afford to do that just yet. But it'll be like, an hour like this, and then a whole day where you can ask me your individual questions

Jul 07, 15:39:11  seacreature: back to tech questions, going to try to hit them in order here

Jul 07, 15:39:15  pleax: volundr: link plz

Jul 07, 15:39:20  volundr: http://github.com/Volundr/rmu-entrance-exam-2010/blob/master/lib/ali_rizvi_fixed_inc.rb

Jul 07, 15:39:37  seacreature: chastell: please restate your question, I'm not sure I understood it

Jul 07, 15:40:03  seacreature: this one < chastell> seacreature: so what are your thoughts on undo/redo callable blocks and current @contents?

Jul 07, 15:40:16  chastell: what I wanted to ask is what's your stance on keeping @contents 'current'

Jul 07, 15:40:24  chastell: and storing the undos in callable blocks

Jul 07, 15:40:39  seacreature: something like

Jul 07, 15:40:49  seacreature: @contents.insert(...); undo { remove_text(args) } ?

Jul 07, 15:41:00  chastell: yeah

Jul 07, 15:41:16  seacreature: Well, it solves pleax's issue, I think. Does it?

Jul 07, 15:41:23  chastell: it does

Jul 07, 15:41:30  pleax: seacreature: I'll take a look

Jul 07, 15:41:46  seacreature: We know the content would be editing in place

Jul 07, 15:41:57  seacreature: and an undo would just be a pop off of the @undos

Jul 07, 15:42:04  seacreature: you'd probably still need to manage a redo stack

Jul 07, 15:42:17  seacreature: and you'd probably still need an execute { } to make things clean, but it could just be

Jul 07, 15:42:23  seacreature: block.call; @reverted = []

Jul 07, 15:42:30  chastell: it's not as concise as the rebuild-contents-on-every-read approach (because you need a redo/undo pairs, yeah), but contents would be always 'current'

Jul 07, 15:42:55  andres-fc: I guess in the end context is king

Jul 07, 15:42:58  neXter:_ very interesting discussion, looking forward to our next one, nice meetin' you all, but as it's nearly 1am I have to go, cu!

Jul 07, 15:43:05  seacreature: Personally, unless I'm missing something, I think that's a nice solution. The danger is that all these lambdas soak up memory like magic

Jul 07, 15:43:10  seacreature: but that's a conversation for another day :)

Jul 07, 15:43:20  locks: my proc solution ate something like 30MB of memory

Jul 07, 15:43:21  chastell: definitely; thanks, no more questions from me :)

Jul 07, 15:43:33  locks: no, more

Jul 07, 15:43:34  locks: 100MB

Jul 07, 15:43:38  seacreature: Okay, rgoytacaz.

Jul 07, 15:43:49  seacreature: You asked about whether it's important to mark objects as private

Jul 07, 15:43:49  fwoeck: cu!

Jul 07, 15:43:54  seacreature: err

Jul 07, 15:43:54  seacreature: methods

Jul 07, 15:43:59  neXter:_ QUITED

Jul 07, 15:44:05  seacreature: With Ruby's send(), we bomb out security immediately

Jul 07, 15:44:07  chastell: (FWIW, my solution for the two actions was at https://gist.github.com/5610ed96c65474b5a243#file_lib_text_edit_naive.rb – cases on hashes instead of callable blocks, as I ran out of time)

Jul 07, 15:44:08  fwoeck: QUITED

Jul 07, 15:44:15  seacreature: so private is in now way, a security feature in Ruby

Jul 07, 15:44:18  seacreature: *no

Jul 07, 15:44:38  andres-fc: it's more a social convention, or to communicate intent with your team

Jul 07, 15:44:42  rgoytacaz: So the general idea is just, "don't bother" ?

Jul 07, 15:44:50  seacreature: rgoytacaz: I sure hope not, not in that way

Jul 07, 15:45:03  seacreature: So I actually have strong feelings about this

Jul 07, 15:45:11  seacreature: In general, you don't want to be testing private methods

Jul 07, 15:45:21  seacreature: which to me, means that private methods should not encapsulate a full behavior

Jul 07, 15:45:25  seacreature: if they do, they should be public

Jul 07, 15:45:34  seacreature: if they don't make sense being public on the object they're on

Jul 07, 15:45:43  seacreature: they should be part of the public API of a lower level object

Jul 07, 15:45:51  seacreature: it's fine to hide that object by #nodoc

Jul 07, 15:46:01  seacreature: and not make it part of the "Public API" of your project

Jul 07, 15:46:16  seacreature: But it's a real object with real behaviors and can be treated like one

Jul 07, 15:46:26  seacreature: makes extensibility, testing, and refactoring much easier

Jul 07, 15:46:49  seacreature: but if you've got a method that's just a label for a large chunk of code, or something that's only job is to perform some side effect state transform

Jul 07, 15:46:52  seacreature: and it's name is like

Jul 07, 15:46:56  seacreature: something_ugly_dont_use_me

Jul 07, 15:47:03  locks: basically, if your design is good there's no need to mark it private explicitly?

Jul 07, 15:47:04  seacreature: then yeah, private it up, and don't worry about testing it

Jul 07, 15:47:20  seacreature: My rule of thumb is that private methods are suspicious

Jul 07, 15:47:42  seacreature: They either mean I need better object composition, or I have some tiny god cropping up in my objecty

Jul 07, 15:47:45  seacreature: *object

Jul 07, 15:47:55  seacreature: But it's pretty plain to see when that's not the case

Jul 07, 15:48:00  seacreature: and in those cases, I don't worry

Jul 07, 15:48:11  seacreature: I make them private so as to not have them called by accident

Jul 07, 15:48:17  rgoytacaz: yup

Jul 07, 15:48:30  seacreature: rgoytacaz: does that deal well with your question?

Jul 07, 15:48:43  rgoytacaz: I did that mainly to remove code duplication, another thing that I saw cropping in some codes

Jul 07, 15:49:00  rgoytacaz: example. @contents.slice(first..last)

Jul 07, 15:49:09  seacreature: rgoytacaz: if you have code duplication, you probably have a behavior you can encapsulate

Jul 07, 15:49:22  rgoytacaz: exactly, thats what I did with private methods

Jul 07, 15:49:23  seacreature: but if it's too insignficant, don't worry about it and just make a private helper

Jul 07, 15:49:25  locks: I cleaned up my solution btw: http://github.com/locks/rmu-entrance-exam-2010/blob/master/lib/ricardo_mendes.rb

Jul 07, 15:49:33  rgoytacaz: private helper?

Jul 07, 15:49:34  ddux: QUITED

Jul 07, 15:49:55  pleax: volundr: yes, for new actions it solves, but for �undo/redo doesn't :)

Jul 07, 15:49:57  seacreature: volundr: I really want to discuss your solution, but I think it'd be better done off line

Jul 07, 15:50:10  seacreature: on the mailing list

Jul 07, 15:50:14  andres-fc: QUITED

Jul 07, 15:50:19  seacreature: pleax: please post your thoughts there

Jul 07, 15:50:33  stuarte: I was going to ask - are helpers a good analogy for the way to use private methods?

Jul 07, 15:50:37  pleax: seacreature: ok, i'll do it tomorrow

Jul 07, 15:50:42  volundr: fair enough :-)

Jul 07, 15:50:52  seacreature: If there is anything I can add to his assessment, I will

Jul 07, 15:51:02  seacreature: pleax apparently has an interest in the actual problem domain

Jul 07, 15:51:10  seacreature: where I'm more interested in exposing a variety of techniques

Jul 07, 15:51:16  seacreature: so he's best to speak on it

Jul 07, 15:51:44  seacreature: stuarte: idk, if you're talking about rails helpers, those are usually a sign of "Wow, Rails sucks, I need to hide some shit"

Jul 07, 15:51:51  carlosantonio: PARTED

Jul 07, 15:51:57  pleax: seacreature: thanks for this meeting :)

Jul 07, 15:52:07  seacreature: and I'll let that be my closing point, happy to elaborate

lines = File.open(ARGV.first).readlines.each_slice(3).map do |time,name,text|
[time,name,text].map(&:chomp)
end
names = lines.map {|line| line[1] }.uniq
lines.each do |line|
text = line[2]
if names.any? {|name| text.start_with? name }
text.sub!( /^([^ :]*:)/, '_\1_' )
end
text.gsub!( %r[(https?://[^ ]*)], '[\1](\1)' )
puts "_#{line[0]}_&nbsp;&nbsp;__#{line[1]}:__ #{text}\n\n"
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment