HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Extracting a set of numbers from a file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filenumbersextractingfromset

Problem

I do most of my programming in C/C++ and Perl, but I currently learning Fortran. I began by coding up a simple program, something that took me < 5 minutes in Perl (see this thread from BioStar). After working for several hours on a solution in Fortran 95 (teaching myself as I go), I came to this solution.

implicit none

! Variable definitions
integer                 :: argc
integer                 :: num_digits
integer                 :: i, j
integer                 :: iocode
integer                 :: line_length
integer                 :: value
character ( len=256 )   :: infile
character ( len=16 )    :: int_format
character ( len=16 )    :: int_string
character ( len=2048 )  :: line

! Verify and parse command line arguments
argc = iargc()
if( argc  0 ) then
  write(*, '(A, A, A)') "Error: unable to open input file '", trim(infile), "'"
  stop
endif

! Process the file, print in CSV format
do while(1 == 1)
  ! Read the line, skip if it is empty
  100 read( unit=1, fmt='(A)', end=200 ) line
  line_length = len_trim(line)
  if( line_length == 0 ) then
    goto 100
  endif

  ! The first value in the line is a string
  ! Find string boundaries and print it out
  i = 0
  do while( line(i:i) == ' ' )
    i = i+1
  end do
  j = i
  do while( line(j:j) /= ' ' )
    j = j+1
  end do
  write(*, '(A)', advance="no") line(i:j-1)

  ! Now grab the rest of the integer values
  ! on the line, multiply them by 3, and print
  i = j
  j = 0
  do while( i < line_length)
    do while( line(i:i) == ' ' )
      i = i+1
    end do
    j = i
    do while( j < line_length .and. line(j:j) /= ' ' )
      j = j+1
    end do
    int_string = line(i:j-1)
    read(int_string, '(I)') value
    value = value*3
    write(*, '(A, I0)', advance="no") ",", value
    i = j
    j = 0
  end do

  print *
end do
200 close( 1 )

end program


There is a lot that I don't like about how I've written this program, but it's hard to separate my inexperience with the new syntax from bad practice. One

Solution

You could get rid of the GOTO statements by using labelled loops with EXIT and CYCLE statements. These allow you to do the sort of flow control that you've used the GOTOs for, but don't permit the less desirable features, such as computed GOTOs. Using these newer Fortran features also gives you a clearer idea of the flow control at the point at which the branching occurs. E.g., CYCLE or EXIT statements send you respectively further up or down the source code, and a well chosen label may indicate what the branching is trying to achieve.

Another suggestion, although this is more of a personal preference, is to avoid the potentially endless DO WHILE(1 == 1) loop by using a normal DO loop with a large upper limit and adding warning message if that limit is reached before the end of file is encountered. Plenty of people might find that overly fussy though.

Example code showing these two points:

PROGRAM so_goto

  IMPLICIT NONE

  INTEGER,PARAMETER :: line_max = 100000

  INTEGER :: i, ios, line_length
  CHARACTER( len=2048 ) :: line

  OPEN(UNIT=10,FILE="so_goto.txt",ACTION="read")

  fread: DO i=1,line_max
    READ(UNIT=10,FMT='(A)',IOSTAT=ios) line
    IF( ios < 0 ) EXIT fread
    line_length = LEN_TRIM(line)
    IF( line_length == 0 ) CYCLE fread

    IF( i == line_max ) THEN
      print*,"BTW, you've hit your read limit before the end of the file."
    ENDIF
  ENDDO fread
  CLOSE(10)

END PROGRAM so_goto

Code Snippets

PROGRAM so_goto

  IMPLICIT NONE

  INTEGER,PARAMETER :: line_max = 100000

  INTEGER :: i, ios, line_length
  CHARACTER( len=2048 ) :: line

  OPEN(UNIT=10,FILE="so_goto.txt",ACTION="read")

  fread: DO i=1,line_max
    READ(UNIT=10,FMT='(A)',IOSTAT=ios) line
    IF( ios < 0 ) EXIT fread
    line_length = LEN_TRIM(line)
    IF( line_length == 0 ) CYCLE fread

    IF( i == line_max ) THEN
      print*,"BTW, you've hit your read limit before the end of the file."
    ENDIF
  ENDDO fread
  CLOSE(10)

END PROGRAM so_goto

Context

StackExchange Code Review Q#145, answer score: 7

Revisions (0)

No revisions yet.