Exporting update

[MichaelRBond]

Exporting updates to support auto loading into an external system.

It is possible that some external systems may need additional information in the control file. However, it should be trivial to add additional control files. Some code will need to be changed. It may be needed to setup an map of replacements for control files. This situation should be addressed IF it ever becomes a concern.

[ddavisgraphics]

Overall Pull Request

Most everything in the PR looks good. I really like how most of the methods are small and that you went for templating in the YAML files. I had a few comments on the YML, but they probably could have been answered by adding more documentation to the Pull Request and a little documentation about the feature in general and how it is going to work.

Is the YAML file going to be dynamically made, or generated other ways. Is YAML the correct ending? What is the YAML file used for in the end or how does the system do the imports into the hydra system? ** Again if you add these in documentation to show how the whole export system is going to work it would be beneficial. Especially if we come into bugs later we know where to start looking.

Code Modifications

    public static function determine_add($field,$type) {
        switch (strtolower($type)){
            case "label":
                return $field['label']; 
            case "name":
                return $field['name']; 
            case "class":
                return $field['class']; 
            case "id":
                return $field['id']; 
            default:
                return "";
        }
    }

The above statement is easier to read in you Export Class and in benchmark testing adds some very minor performance benefits.

There is no documentation on the items your adding and no documentation on anything in the class. Please use documentation to give expections of your methods in your class.

/**
     * Describe what is going on in the method.
     *
     * @param string $prefix describes what the parameter is expected to do or the expected type of the param.  
     * @return what is expected to be returned use void if nothing
     */

Could you also place documentation in on how the exports need to work or criteria to use them. I know this all "mundane and tedious", but overall any contribution that anyone can't quickly understand doesn't do us much good.

[MichaelRBond]

yaml file extension is fine. i is more descriptive than yml, and all mdern operating systems can handle more than 3 character file extensions.

[MichaelRBond]

adding another function, in place of the regular expressions is ridiculous. its adding another function call, where it is absolutely not needed, less concise, more difficult to read (because you have to bounce around), and harder to maintain in the long run. I will not make that change.

If you would like that change, go ahead and make it. but the first time i have to maintain that code, i wiil revert it your commit, and start from there.

[MichaelRBond]

I'll add documentation to the wiki when the process is done.

[ddavisgraphics]

@MichaelRBond Please show me how this

function determine_add($field,$type) {
        if (strtolower($type) == "label") {
            return $field['label'];
        }
        else if (strtolower($type) == "name") {
            return $field['name'];
        }
        else if (strtolower($type) == "class") {
            return $field['class'];
        }
        else if (strtolower($type) == "id") {
            return $field['id'];
        }
        return "";
    }

is more readable than

 public static function determine_add($field,$type) {
        switch (strtolower($type)){
            case "label":
                return $field['label']; 
            case "name":
                return $field['name']; 
            case "class":
                return $field['class']; 
            case "id":
                return $field['id']; 
            default:
                return "";
        }
    }

I don't know what your talking about adding more functions for regular expressions.

[MichaelRBond]

I was taking a guess, because you didn't reply with a line comment. Maybe I guessed wrong.

Please reply with line comments

Get Outlook for iOShttps://aka.ms/o0ukef

---

From: David J. Davis notifications@github.com
Sent: Monday, November 14, 2016 12:35:16 PM
To: wvulibraries/mfcs
Cc: Michael Bond; Mention
Subject: Re: [wvulibraries/mfcs] Exporting update (#244)

@MichaelRBondhttps://github.com/MichaelRBond That is crazy to assume that

function determine_add($field,$type) {
if (strtolower($type) == "label") {
return $field['label'];
}
else if (strtolower($type) == "name") {
return $field['name'];
}
else if (strtolower($type) == "class") {
return $field['class'];
}
else if (strtolower($type) == "id") {
return $field['id'];
}
return "";
}

is more readable than

public static function determine_add($field,$type) {
switch (strtolower($type)){
case "label":
return $field['label'];
case "name":
return $field['name'];
case "class":
return $field['class'];
case "id":
return $field['id'];
default:
return "";
}
}

I don't know what your talking about adding more functions for regular expressions.

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://github.com//pull/244#issuecomment-260403733, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABadrIkLSPUDQvzTEiVq4W1CyXtcz1ofks5q-JvUgaJpZM4KwrMR.

[ddavisgraphics]

It is line 39 on exporting, but I guess it isn't a changed part of the file in your main PR so that can be adjusted later. Still the main issues are with documentation of methods and documentation on how it is supposed to work and what it accomplishes.