Skip to content
This repository has been archived by the owner on May 10, 2018. It is now read-only.

A bit DRYer #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

A bit DRYer #3

wants to merge 1 commit into from

Conversation

devboy
Copy link

@devboy devboy commented Apr 9, 2011

Just saw this and thought I make a quick change. Not tested, just wrote it in TextMate without compiling.

@devboy
Copy link
Author

devboy commented Apr 9, 2011

I would also propose to remove the different sendThis sendThat functions in the connectors and use a function that takes for example a LogType.WARN or LogType.INFO as a parameter.

@jankeesvw
Copy link
Owner

Hi,

Thanks for the pull request, i like your change but i think it makes the code harder to read.. don't you agree? But I think most people use the .swc anyways..

What do you mean with the second comment?

@devboy
Copy link
Author

devboy commented Apr 10, 2011

I think you could shrink down all your connector classes and maybe others to just a few lines of code, by removing all the code that is basically copy & paste.
And introducing MessageType in some sort, would make the code more readable yet smaller and DRYer. imho.

@devboy
Copy link
Author

devboy commented Apr 10, 2011

Additonaly I saw that there is no need to extend the AbstractConnector. I would rather use it in a compositional sense than an abstract.

@jankeesvw
Copy link
Owner

I totally agree with you, but the code needs to be really readable :-)

The AbstractConnector is pretty lame, a static function to get the sender would be enough!

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

Successfully merging this pull request may close these issues.

2 participants