Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 13 additions & 5 deletions Gopkg.toml
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@

# Gopkg.toml example
#
# Refer to https://github.com/golang/dep/blob/master/docs/Gopkg.toml.md
# Refer to https://golang.github.io/dep/docs/Gopkg.toml.html
# for detailed Gopkg.toml documentation.
#
# required = ["github.com/user/thing/cmd/thing"]
required = ["github.com/CamilleEscher/clever-challenge/"]
# ignored = ["github.com/user/project/pkgX", "bitbucket.org/user/project/pkgA/pkgY"]
#
# [[constraint]]
Expand All @@ -17,6 +16,15 @@
# source = "github.com/myfork/project2"
#
# [[override]]
# name = "github.com/x/y"
# version = "2.4.0"
# name = "github.com/x/y"
# version = "2.4.0"
#
# [prune]
# non-go = false
# go-tests = true
# unused-packages = true


[prune]
go-tests = true
unused-packages = true
139 changes: 138 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
// go run result.go main.go
package main

import (
"fmt"
"time"
"strings"
"bufio"
"os"
"io/ioutil"
"log"
"path"
"regexp"
)

//timeTrack tracks the time it took to do things.
Expand Down Expand Up @@ -32,6 +40,135 @@ func main() {
// number of line deleted
// list of function calls seen in the diffs and their number of calls
func compute() *result {
const diff_dir_path = "./diffs/"
files, err := ioutil.ReadDir(diff_dir_path)
if err != nil {
log.Fatal(err)
}
res := result{}
for _, f := range files {
file_path := path.Join(diff_dir_path, f.Name())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Watch out with the path package as it is not intended for file system path manipulation, filepath is more suited for this

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted.

get_data(&res, &file_path)
}
return &res
}

func get_data(res *result, file_path *string) {
file, err := os.Open(*file_path)
defer file.Close()
if err != nil {
log.Fatal(err)
}
scanner := bufio.NewScanner(file)

task_list := []string{"DIFF", "REGION", "UPDATE"}
for scanner.Scan() {
if scanner.Text() != "" {
if len(scanner.Text()) > 0 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I speak for myself, and possibly others will agree that this can be &ed for more linear code, this'll help readability

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree.

for _, task := range task_list {
success, on_success := execute_task(&task, scanner.Text(), res)
if success && len(on_success) == 0 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be simplified by the negative; i.e. if !success { continue }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it works

continue
} else if success {
task_list = on_success

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand what you are doing, is it some state machine?
It may require a bit of commenting since it was rather confusing for me at first.

Copy link
Author

@CamilleEscher CamilleEscher Sep 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is kind of a state machine where I have mainly tree states, the states determine which pattern to search in priority, like a very simple waterfall where we test the most complex and intricated task when no other task succeded, the tasks being finding clue about state transition, states 'being in a diff headline' (DIFF), 'in a region headline' (REGION) 'in diff content' (UPDATE). When we are in state UPDATE, we have to check all states with priority DIFF then REGION then UPDATE and FUNCTION CALL.

break
}
}
}
}
}
if err := scanner.Err(); err != nil {
log.Fatal(err)
}
}

func execute_task(task *string, line string, res *result) (bool, []string) {
success := false
on_success := make([]string, 0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a small detail, but this should be declared as nil first

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

if *task == "DIFF" {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going towards a state-machine approach, I'd suggest using enums for that kind of manipulation, it'll be way lighter than strings for states.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I just found out how to write enums.

if is_new_diff(&line, res) {
success = true
on_success = append(on_success, "REGION")
}
} else if *task == "REGION" {
if is_new_region(&line, res) {
success = true
on_success = append(on_success, "DIFF", "REGION", "UPDATE")
}
} else if *task == "UPDATE" {
parse_line_update(line[0], res)
// is function
if c_code(res) {
c_func_pattern := regexp.MustCompile(`[_a-zA-Z]+[_a-zA-Z0-9]*\w*\(.*\)[\w/]*;`)
match := c_func_pattern.FindAllStringIndex(line, -1)
for _, val := range match {
func_name := line[val[0] : val[1]]
opening_parenthesis := regexp.MustCompile(`\(`)
match_par := opening_parenthesis.FindStringIndex(func_name)
if len(match_par) == 2 {
func_name := line[val[0] : val[0] + match_par[0]]
if _, ok := res.functionCalls[func_name]; ok {
res.functionCalls[func_name]++
} else if res.functionCalls != nil {
res.functionCalls[func_name] = 1
} else {
res.functionCalls = map[string]int{func_name : 1}
}
}
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a fallback for missing future cases may be worth it

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}
return success, on_success
}

func c_code(res *result) bool {
has_c_ext := false
if len(res.files) > 0 {
if path.Ext(res.files[len(res.files) - 1]) == ".c" {
has_c_ext = true
}
}
return has_c_ext
}

// Extract $file_path_a and $file_path_b if line = 'diff --git a/$file_path_a b/$file_path_b'
func is_new_diff(line *string, res *result) bool {
new_diff := false
if line != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is personal preference (and is something advocated by several open-source projects), but I think you should linearize your code for easier understanding of what's happening.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I see that I refined a lot of conditions and indeed there refacto to do.

if len(*line) > 10 {
if (*line)[:11] == "diff --git " {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both those lines could be replaced by a strings.HasPrefix

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It increased a bit performances.

file_a_pattern := regexp.MustCompile(`a/.* b/`)
match := file_a_pattern.FindStringIndex(*line)
if len(match) == 2 {
file_a := (*line)[match[0] + 2 : match[1] - 3]
file_b := strings.TrimSpace((*line)[match[1] :])
res.files = append(res.files, file_a, file_b)
new_diff = true
}
}
}
}
return new_diff
}

// region pattern '@@ -127,3 +127,7 @@'
func is_new_region(line *string, res *result) bool {
new_region := false
stat_pattern := regexp.MustCompile(`\@\@ \-\d+,\d+ \+\d+,\d+ @@`)
indices := stat_pattern.FindStringIndex(*line)
if len(indices) == 2 {
res.regions += 1
new_region = true
}
return new_region
}

return nil
// Track reference of '+' or '-' in front_char of a line
// to indicate added / removed lines in diff
func parse_line_update(front_char byte, res *result) {
if front_char == '+' {
res.lineAdded += 1
} else if front_char == '-' {
res.lineDeleted += 1
}
}