I like the idea of this change. Using key and value makes things more readable at the site of use than a generic first/second. It's unfortunate that it is hard to deploy, but I think it's worth it to work through those challenges. `
-Maciej On Aug 28, 2012, at 3:24 PM, Caio Marcelo de Oliveira Filho <caio.olive...@openbossa.org> wrote: > Hello, > > I would like to propose we change HashMap iterators to use 'key' and > 'value' instead of 'first' and 'second'. The work is being tracked in > the bug https://bugs.webkit.org/show_bug.cgi?id=82784. > > The idea came from https://bugs.webkit.org/show_bug.cgi?id=71063#c2. I > argue that this change will make code more readable, specially in > situations where either the key or the value is a pair. One example of > this situation is in > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp > > GetterSetterMap::AddResult result = > map.add(node->name().impl(), pair); > if (!result.isNewEntry) > - result.iterator->second.second = node; > + result.iterator->value.second = node; > > another one is in > Source/WebCore/platform/graphics/chromium/TiledLayerChromium.cpp > > for (CCLayerTilingData::TileMap::const_iterator iter = > m_tiler->tiles().begin(); iter != m_tiler->tiles().end(); ++iter) { > - int i = iter->first.first; > - int j = iter->first.second; > - UpdatableTile* tile = > static_cast<UpdatableTile*>(iter->second.get()); > + int i = iter->key.first; > + int j = iter->key.second; > + UpdatableTile* tile = static_cast<UpdatableTile*>(iter->value.get()); > > I also think that in "non-pair key and non-pair value" situations the > readability is improved. > > One downside of this change is that STL uses first and second in its > maps so that would confuse people. After inspecting the whole > repository for 'first' and 'second', the usage of std::map in WebKit > is an exception not a rule, IMHO this makes the confusion less > troublesome. > > Are there other downsides am I missing? What do you guys think? Unless > we figure this is a bad change for the project, I intend to land it. > > I've attempted to land this previously, I found out it caused a major > breakage in some internal builds. I apologize for that inconvenience > and the time lost. > > I'm looking into ways to make the transition smoother, but until now > couldn't find a solution that doesn't require C++11. Either way, if we > land this change, I'll sync with developers from the affected ports > that I know could be internally affected to avoid more breakage. > > > Best regards, > > -- > Caio Marcelo de Oliveira Filho > openBossa @ INdT - Instituto Nokia de Tecnologia > _______________________________________________ > webkit-dev mailing list > webkit-dev@lists.webkit.org > http://lists.webkit.org/mailman/listinfo/webkit-dev _______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev