mongodb-labs / edda

A log visualizer for MongoDB - This Repository is NOT a supported MongoDB product
232 stars 28 forks source link

[cleanup] refactoring #66

Closed kchodorow closed 12 years ago

kchodorow commented 12 years ago

This is minor, don't bother pre-.1.

https://github.com/kchodorow/logl/blob/master/logl/post/event_matchup.py#L222-257 should be refactored into a function.

kaushal commented 12 years ago

Could you specify what you want for this issue? This link seems to be broken.

kchodorow commented 12 years ago

New link: https://github.com/kchodorow/edda/blob/master/edda/post/event_matchup.py#L229-267

There's a bunch of code that does the same thing for a & b, should be refactored into a function so target_server_match can call func(a) and func(b).

kaushal commented 12 years ago

Is this really necessary? The point of this method is to compare the results of a and b. The only distinct things for a and b are: a = entry_a["info"]["server"] b = entry_b["info"]["server"]

if a == "self" and b == "self":
    return False
if a == b:
    return True
a_doc = servers.find_one({"server_num": entry_a["origin_server"]})
b_doc = servers.find_one({"server_num": entry_b["origin_server"]})

I could make functions out of these operations, but it wouldn't save us any run time. And the next block of code compares the results of a and b:

if a == "self" and b == a_doc["network_name"]:
        return True
if b == "self" and a == b_doc["network_name"]:
        return True

# address not known
# in this case, we will assume that the address does belong
# to the unnamed server and name it.
if a == "self":
    if a_doc["network_name"] == "unknown":
        LOGGER.info("Assigning network name {0} to server {1}".format(b, a))
        a_doc["network_name"] == b
        servers.save(a_doc)
        return True
    return False

if b == "self":
    if b_doc["network_name"] == "unknown":
        LOGGER.info("Assigning network name {0} to server {1}".format(a, b))
        b_doc["network_name"] == a
        servers.save(b_doc)
        return True
    return False
kchodorow commented 12 years ago

The "if a == "self":" and "if b=="self":" blocks were what I was thinking of in particular. They are doing the same thing twice in a row.

It's not as much about run time as it is code readability.

kaushal commented 12 years ago

Ok, I could do that.