PicasaAlbumTableReader for Media Galleries API

webkit/fileapi/media/picasa/pmp_table_reader.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/fileapi/media/picasa/pmp_table_reader.h"
 
 #include <algorithm>
 
 #include "base/file_util.h"
-#include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "webkit/fileapi/media/picasa/pmp_column_reader.h"
-#include "webkit/fileapi/media/picasa/pmp_constants.h"
 
 namespace picasaimport {
 
 namespace {
 
 COMPILE_ASSERT(sizeof(double) == 8, double_must_be_8_bytes_long);
 
 }  // namespace
 
-PmpTableReader::PmpTableReader() : column_readers_(), max_row_count_(0) { }
+PmpTableReader::PmpTableReader(const std::string& table_name,
+                               const base::FilePath& directory_path)
+    : table_name_(table_name),
+      directory_path_(directory_path),
+      column_readers_(),
+      max_row_count_(0) { }
 
 PmpTableReader::~PmpTableReader() { }
 
-bool PmpTableReader::Init(const std::string& table_name,
-                          const base::FilePath& directory_path,
-                          const std::vector<std::string>& columns) {
-  DCHECK(!columns.empty());
-
-  if (!column_readers_.empty())
+bool PmpTableReader::Init() {

[vandebo (ex-Chrome), 2013/04/08 23:57:24]

Maybe just put the contents on Init into the constructor.  If init fails, then
set some state that you can detect in AddColumn, then fail immediately in
AddColumn if Init would have failed ?

[tommycli, 2013/04/09 00:45:34]

I just added a 
  base::ThreadRestrictions::AssertIOAllowed();
to Init(). Is that safe to put in a constructor? Should I just add a boolean
member to keep track of whether or not construction succeeded?

[vandebo (ex-Chrome), 2013/04/09 19:16:56]

If the compiler is happy, I'm happy.

A Boolean is fine unless there's another member that you want to overload.

[tommycli, 2013/04/11 18:24:05]

Done.

+  if (!file_util::DirectoryExists(directory_path_))
     return false;
 
-  if (!file_util::DirectoryExists(directory_path))
-    return false;
-
-  std::string table_prefix = table_name + "_";
-  std::string indicator_file_name = table_prefix + "0";
+  std::string indicator_file_name = table_name_ + "_0";
 
 #if defined(OS_WIN)
-  base::FilePath indicator_file = directory_path.Append(
+  base::FilePath indicator_file = directory_path_.Append(
       UTF8ToUTF16(indicator_file_name));
 #else
   base::FilePath indicator_file =
-      directory_path.Append(indicator_file_name);
+      directory_path_.Append(indicator_file_name);
 #endif
 
   // Look for the indicator_file file, indicating table existence.
-  if (!file_util::PathExists(indicator_file) ||
-      file_util::DirectoryExists(indicator_file)) {
-    return false;
-  }
+  return file_util::PathExists(indicator_file) &&
+         !file_util::DirectoryExists(indicator_file);
+}
 
-  ScopedVector<PmpColumnReader> column_readers;
-  uint32 max_row_count = 0;
-
-  for (std::vector<std::string>::const_iterator it = columns.begin();
-       it != columns.end(); ++it) {
-    std::string filename = table_prefix + *it + "." + kPmpExtension;
+const PmpColumnReader* PmpTableReader::AddColumn(
+    const std::string& column_name, const PmpFieldType expected_type) {
+  std::string filename = table_name_ + "_" + column_name + "." + kPmpExtension;
 
 #if defined(OS_WIN)
-    base::FilePath column_file_path = directory_path.Append(
-        UTF8ToUTF16(filename));
+  base::FilePath column_file_path = directory_path_.Append(
+      UTF8ToUTF16(filename));
 #else
-    base::FilePath column_file_path = directory_path.Append(filename);
+  base::FilePath column_file_path = directory_path_.Append(filename);
 #endif
 
+  scoped_ptr<PmpColumnReader> column_reader(new PmpColumnReader());
 
-    PmpColumnReader* column_reader = new PmpColumnReader();
-    column_readers.push_back(column_reader);
+  uint32 row_count;
+  if (!column_reader->Init(column_file_path, expected_type, &row_count))
+    return NULL;
 
-    uint32 row_cnt;
+  column_readers_.push_back(column_reader.release());
 
-    if (!column_reader->Init(column_file_path, &row_cnt))
-      return false;
+  max_row_count_ = std::max(max_row_count_, row_count);
 
-    max_row_count = std::max(max_row_count, row_cnt);
-  }
-
-  column_readers_ = column_readers.Pass();
-  max_row_count_ = max_row_count;
-
-  return true;
+  return column_readers_.back();
 }
 
 uint32 PmpTableReader::RowCount() const {
   return max_row_count_;
 }
 
 std::vector<const PmpColumnReader*> PmpTableReader::GetColumns() const {
   std::vector<const PmpColumnReader*> readers(
       column_readers_.begin(), column_readers_.end());
   return readers;
 }
 
 }  // namespace picasaimport